From 478f7611884d4a5c065b79705c588ed4b1eeae01 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 11:19:12 +0000 Subject: [PATCH 01/23] Add Spree::Gateway::PayPalExpress to spree payment_methods list --- config/application.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/application.rb b/config/application.rb index d69b4cdf34..e48ce0556b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -129,6 +129,7 @@ module Openfoodnetwork app.config.spree.payment_methods << Spree::Gateway::Pin app.config.spree.payment_methods << Spree::Gateway::StripeConnect app.config.spree.payment_methods << Spree::Gateway::StripeSCA + app.config.spree.payment_methods << Spree::Gateway::PayPalExpress end # Settings in config/environments/* take precedence over those specified here. From 2fb7dfa4300a762441bbefbb9631f2b0cc0a7df4 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 11:39:49 +0000 Subject: [PATCH 02/23] Bring in Paypal Express javascript --- .../javascripts/admin/spree_paypal_express.js | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 app/assets/javascripts/admin/spree_paypal_express.js 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 0000000000..7bdf9b187e --- /dev/null +++ b/app/assets/javascripts/admin/spree_paypal_express.js @@ -0,0 +1,25 @@ +//= require admin/spree_backend + +SpreePaypalExpress = { + hideSettings: function(paymentMethod) { + if (SpreePaypalExpress.paymentMethodID && paymentMethod.val() == SpreePaypalExpress.paymentMethodID) { + $('.payment-method-settings').children().hide(); + $('#payment_amount').prop('disabled', 'disabled'); + $('button[type="submit"]').prop('disabled', 'disabled'); + $('#paypal-warning').show(); + } else if (SpreePaypalExpress.paymentMethodID) { + $('.payment-method-settings').children().show(); + $('button[type=submit]').prop('disabled', ''); + $('#payment_amount').prop('disabled', '') + $('#paypal-warning').hide(); + } + } +} + +$(document).ready(function() { + checkedPaymentMethod = $('[data-hook="payment_method_field"] input[type="radio"]:checked'); + SpreePaypalExpress.hideSettings(checkedPaymentMethod); + paymentMethods = $('[data-hook="payment_method_field"] input[type="radio"]').click(function (e) { + SpreePaypalExpress.hideSettings($(e.target)); + }); +}) From 42468e2ef33f72e39e9189f5e411534980de916e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 11:48:54 +0000 Subject: [PATCH 03/23] Apply Spree::Admin::PaymentsController decorator --- .../spree/admin/payments_controller.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/app/controllers/spree/admin/payments_controller.rb b/app/controllers/spree/admin/payments_controller.rb index 8b5f42c472..e0a530a823 100644 --- a/app/controllers/spree/admin/payments_controller.rb +++ b/app/controllers/spree/admin/payments_controller.rb @@ -67,6 +67,25 @@ module Spree redirect_to request.referer end + def paypal_refund + if request.get? + if @payment.source.state == 'refunded' + flash[:error] = Spree.t(:already_refunded, scope: 'paypal') + redirect_to admin_order_payment_path(@order, @payment) + end + elsif request.post? + response = @payment.payment_method.refund(@payment, params[:refund_amount]) + if response.success? + flash[:success] = Spree.t(:refund_successful, scope: 'paypal') + redirect_to admin_order_payments_path(@order) + else + flash.now[:error] = Spree.t(:refund_unsuccessful, scope: 'paypal') + + " (#{response.errors.first.long_message})" + render + end + end + end + private def load_payment_source From 597eed5285a7cd05e4f7ea77afefd920db9c8c47 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 11:51:50 +0000 Subject: [PATCH 04/23] Bring in Spree::Admin::PaypalPaymentsController --- .../spree/admin/paypal_payments_controller.rb | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 app/controllers/spree/admin/paypal_payments_controller.rb diff --git a/app/controllers/spree/admin/paypal_payments_controller.rb b/app/controllers/spree/admin/paypal_payments_controller.rb new file mode 100644 index 0000000000..872a23d6d7 --- /dev/null +++ b/app/controllers/spree/admin/paypal_payments_controller.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Spree + module Admin + class PaypalPaymentsController < Spree::Admin::BaseController + before_action :load_order + + def index + @payments = @order.payments.includes(:payment_method). + where(spree_payment_methods: { type: "Spree::Gateway::PayPalExpress" }) + end + + private + + def load_order + @order = Spree::Order.where(number: params[:order_id]).first + end + end + end +end From 94549e98ba1cacbd6073c37103f93092eda20575 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 11:54:41 +0000 Subject: [PATCH 05/23] Bring in Spree::PaypalController --- app/controllers/spree/paypal_controller.rb | 183 +++++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 app/controllers/spree/paypal_controller.rb diff --git a/app/controllers/spree/paypal_controller.rb b/app/controllers/spree/paypal_controller.rb new file mode 100644 index 0000000000..b629f731d4 --- /dev/null +++ b/app/controllers/spree/paypal_controller.rb @@ -0,0 +1,183 @@ +# frozen_string_literal: true + +module Spree + class PaypalController < StoreController + ssl_allowed + + def express + order = current_order || raise(ActiveRecord::RecordNotFound) + items = order.line_items.map(&method(:line_item)) + + tax_adjustments = order.adjustments.tax + # TODO: Remove in Spree 2.2 + tax_adjustments = tax_adjustments.additional if tax_adjustments.respond_to?(: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 #10 + # https://cms.paypal.com/uk/cgi-bin/?cmd=_render-content&content_ID=developer/e_howto_api_ECCustomizing + # "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)) + + begin + pp_response = provider.set_express_checkout(pp_request) + if pp_response.success? + redirect_to provider.express_checkout_url(pp_response, :useraction => 'commit') + else + flash[:error] = Spree.t('flash.generic_error', :scope => 'paypal', :reasons => pp_response.errors.map(&:long_message).join(" ")) + redirect_to checkout_state_path(:payment) + end + rescue SocketError + flash[:error] = Spree.t('flash.connection_failed', :scope => 'paypal') + redirect_to checkout_state_path(:payment) + end + end + + def confirm + order = current_order || raise(ActiveRecord::RecordNotFound) + order.payments.create!({ + :source => Spree::PaypalExpressCheckout.create({ + :token => params[:token], + :payer_id => params[:PayerID] + } ), + :amount => order.total, + :payment_method => payment_method + } ) + order.next + if order.complete? + flash.notice = Spree.t(:order_processed_successfully) + flash[:commerce_tracking] = "nothing special" + session[:order_id] = nil + redirect_to completion_route(order) + else + redirect_to checkout_state_path(order.state) + end + end + + def cancel + flash[:notice] = Spree.t('flash.cancel', :scope => 'paypal') + order = current_order || raise(ActiveRecord::RecordNotFound) + redirect_to checkout_state_path(order.state, paypal_cancel_token: params[:token]) + end + + 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 + { :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)] + }} + end + + def payment_method + Spree::PaymentMethod.find(params[:payment_method_id]) + end + + def provider + payment_method.provider + end + + def payment_details items + item_sum = items.sum { |i| i[:Quantity] * i[:Amount][:value] } + # Would use tax_total here, but it can include "included" taxes as well. + # For instance, tax_total would include the 10% GST in Australian stores. + # A quick sum will get us around that little problem. + # TODO: Remove additional check in 2.2 + tax_adjustments = current_order.adjustments.tax + tax_adjustments = tax_adjustments.additional if tax_adjustments.respond_to?(:additional) + tax_adjustments_total = tax_adjustments.sum(:amount) + + if item_sum.zero? + # Paypal does not support no items or a zero dollar ItemTotal + # This results in the order summary being simply "Current purchase" + { + :OrderTotal => { + :currencyID => current_order.currency, + :value => current_order.total + } + } + else + { + :OrderTotal => { + :currencyID => current_order.currency, + :value => current_order.total + }, + :ItemTotal => { + :currencyID => current_order.currency, + :value => item_sum + }, + :ShippingTotal => { + :currencyID => current_order.currency, + :value => current_order.ship_total + }, + :TaxTotal => { + :currencyID => current_order.currency, + :value => tax_adjustments_total, + }, + :ShipToAddress => address_options, + :PaymentDetailsItem => items, + :ShippingMethod => "Shipping Method Name Goes Here", + :PaymentAction => "Sale" + } + end + end + + def address_options + return {} unless address_required? + + { + :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 + } + end + + def completion_route(order) + order_path(order, :token => order.token) + end + + def address_required? + payment_method.preferred_solution.eql?('Sole') + end + end +end From 4ca3e29458fdde3efca4d19663d7fdee22a87e36 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 12:03:33 +0000 Subject: [PATCH 06/23] Apply changes from and remove Spree::PaypalController decorator --- app/controllers/spree/paypal_controller.rb | 139 +++++++++++----- .../spree/paypal_controller_decorator.rb | 157 ------------------ 2 files changed, 99 insertions(+), 197 deletions(-) delete mode 100644 app/controllers/spree/paypal_controller_decorator.rb diff --git a/app/controllers/spree/paypal_controller.rb b/app/controllers/spree/paypal_controller.rb index b629f731d4..4d1aaa465a 100644 --- a/app/controllers/spree/paypal_controller.rb +++ b/app/controllers/spree/paypal_controller.rb @@ -4,6 +4,13 @@ module Spree class PaypalController < StoreController ssl_allowed + include OrderStockCheck + + before_action :enable_embedded_shopfront + before_action :destroy_orphaned_paypal_payments, only: :confirm + after_action :reset_order_when_complete, only: :confirm + before_action :permit_parameters! + def express order = current_order || raise(ActiveRecord::RecordNotFound) items = order.line_items.map(&method(:line_item)) @@ -15,12 +22,13 @@ module Spree 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 + Name: adjustment.label, + Quantity: 1, + Amount: { + currencyID: order.currency, + value: adjustment.amount } } end @@ -37,42 +45,57 @@ module Spree begin pp_response = provider.set_express_checkout(pp_request) if pp_response.success? - redirect_to provider.express_checkout_url(pp_response, :useraction => 'commit') + # At this point Paypal has *provisionally* accepted that the payment can now be placed, + # and the user will be redirected to a Paypal payment page. On completion, the user is + # sent back and the response is handled in the #confirm action in this controller. + redirect_to provider.express_checkout_url(pp_response, useraction: 'commit') else - flash[:error] = Spree.t('flash.generic_error', :scope => 'paypal', :reasons => pp_response.errors.map(&:long_message).join(" ")) - redirect_to checkout_state_path(:payment) + flash[:error] = Spree.t('flash.generic_error', scope: 'paypal', reasons: pp_response.errors.map(&:long_message).join(" ")) + redirect_to spree.checkout_state_path(:payment) end rescue SocketError - flash[:error] = Spree.t('flash.connection_failed', :scope => 'paypal') - redirect_to checkout_state_path(:payment) + flash[:error] = Spree.t('flash.connection_failed', scope: 'paypal') + redirect_to spree.checkout_state_path(:payment) end end def confirm - order = current_order || raise(ActiveRecord::RecordNotFound) - order.payments.create!({ - :source => Spree::PaypalExpressCheckout.create({ - :token => params[:token], - :payer_id => params[:PayerID] - } ), - :amount => order.total, - :payment_method => payment_method - } ) - order.next - if order.complete? + @order = current_order || raise(ActiveRecord::RecordNotFound) + + # At this point the user has come back from the Paypal form, and we get one + # last chance to interact with the payment process before the money moves... + return reset_to_cart unless sufficient_stock? + + @order.payments.create!({ + source: Spree::PaypalExpressCheckout.create({ + token: params[:token], + payer_id: params[:PayerID] + }), + amount: @order.total, + payment_method: payment_method + }) + @order.next + if @order.complete? flash.notice = Spree.t(:order_processed_successfully) flash[:commerce_tracking] = "nothing special" session[:order_id] = nil - redirect_to completion_route(order) + redirect_to completion_route(@order) else - redirect_to checkout_state_path(order.state) + redirect_to checkout_state_path(@order.state) end end def cancel - flash[:notice] = Spree.t('flash.cancel', :scope => 'paypal') - order = current_order || raise(ActiveRecord::RecordNotFound) - redirect_to checkout_state_path(order.state, paypal_cancel_token: params[:token]) + flash[:notice] = Spree.t('flash.cancel', scope: 'paypal') + redirect_to main_app.checkout_path + end + + # Clears the cached order. Required for #current_order to return a new order + # to serve as cart. See https://github.com/spree/spree/blob/1-3-stable/core/lib/spree/core/controller_helpers/order.rb#L14 + # for details. + def expire_current_order + session[:order_id] = nil + @current_order = nil end private @@ -90,22 +113,58 @@ module Spree } 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)] - }} + def express_checkout_request_details(order, items) + { + SetExpressCheckoutRequestDetails: { + InvoiceID: order.number, + BuyerEmail: order.email, + ReturnURL: spree.confirm_paypal_url(payment_method_id: params[:payment_method_id], utm_nooverride: 1), + CancelURL: spree.cancel_paypal_url, + SolutionType: payment_method.preferred_solution.presence || "Mark", + LandingPage: payment_method.preferred_landing_page.presence || "Billing", + cppheaderimage: payment_method.preferred_logourl.presence || "", + NoShipping: 1, + PaymentDetails: [payment_details(items)] + } + } end def payment_method - Spree::PaymentMethod.find(params[:payment_method_id]) + @payment_method ||= Spree::PaymentMethod.find(params[:payment_method_id]) + end + + def permit_parameters! + params.permit(:token, :payment_method_id, :PayerID) + end + + def reset_order_when_complete + if current_order.complete? + flash[:notice] = t(:order_processed_successfully) + + OrderCompletionReset.new(self, current_order).call + session[:access_token] = current_order.token + end + end + + def reset_to_cart + OrderCheckoutRestart.new(@order).call + handle_insufficient_stock + end + + # See #1074 and #1837 for more detail on why we need this + # An 'orphaned' Spree::Payment is created for every call to CheckoutController#update + # for orders that are processed using a Spree::Gateway::PayPalExpress payment method + # These payments are 'orphaned' because they are never used by the spree_paypal_express gem + # which creates a brand new Spree::Payment from scratch in PayPalController#confirm + # However, the 'orphaned' payments are useful when applying a transaction fee, because the fees + # need to be calculated before the order details are sent to PayPal for confirmation + # This is our best hook for removing the orphaned payments at an appropriate time. ie. after + # the payment details have been confirmed, but before any payments have been processed + def destroy_orphaned_paypal_payments + return unless payment_method.is_a?(Spree::Gateway::PayPalExpress) + + orphaned_payments = current_order.payments.where(payment_method_id: payment_method.id, source_id: nil) + orphaned_payments.each(&:destroy) end def provider @@ -173,7 +232,7 @@ module Spree end def completion_route(order) - order_path(order, :token => order.token) + spree.order_path(order, token: order.token) end def address_required? diff --git a/app/controllers/spree/paypal_controller_decorator.rb b/app/controllers/spree/paypal_controller_decorator.rb deleted file mode 100644 index 97fedf74af..0000000000 --- a/app/controllers/spree/paypal_controller_decorator.rb +++ /dev/null @@ -1,157 +0,0 @@ -# frozen_string_literal: true - -Spree::PaypalController.class_eval do - include OrderStockCheck - - before_action :enable_embedded_shopfront - before_action :destroy_orphaned_paypal_payments, only: :confirm - after_action :reset_order_when_complete, only: :confirm - before_action :permit_parameters! - - def express - order = current_order || raise(ActiveRecord::RecordNotFound) - items = order.line_items.map(&method(:line_item)) - - tax_adjustments = order.adjustments.tax - # TODO: Remove in Spree 2.2 - tax_adjustments = tax_adjustments.additional if tax_adjustments.respond_to?(: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 #10 - # https://cms.paypal.com/uk/cgi-bin/?cmd=_render-content&content_ID=developer/e_howto_api_ECCustomizing - # "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)) - - begin - pp_response = provider.set_express_checkout(pp_request) - if pp_response.success? - # At this point Paypal has *provisionally* accepted that the payment can now be placed, - # and the user will be redirected to a Paypal payment page. On completion, the user is - # sent back and the response is handled in the #confirm action in this controller. - redirect_to provider.express_checkout_url(pp_response, useraction: 'commit') - else - flash[:error] = Spree.t('flash.generic_error', scope: 'paypal', reasons: pp_response.errors.map(&:long_message).join(" ")) - redirect_to spree.checkout_state_path(:payment) - end - rescue SocketError - flash[:error] = Spree.t('flash.connection_failed', scope: 'paypal') - redirect_to spree.checkout_state_path(:payment) - end - end - - def confirm - @order = current_order || raise(ActiveRecord::RecordNotFound) - - # At this point the user has come back from the Paypal form, and we get one - # last chance to interact with the payment process before the money moves... - return reset_to_cart unless sufficient_stock? - - @order.payments.create!( - source: Spree::PaypalExpressCheckout.create( - token: params[:token], - payer_id: params[:PayerID] - ), - amount: @order.total, - payment_method: payment_method - ) - @order.next - if @order.complete? - flash.notice = Spree.t(:order_processed_successfully) - flash[:commerce_tracking] = "nothing special" - session[:order_id] = nil - redirect_to completion_route(@order) - else - redirect_to checkout_state_path(@order.state) - end - end - - def cancel - flash[:notice] = Spree.t('flash.cancel', scope: 'paypal') - redirect_to main_app.checkout_path - end - - # Clears the cached order. Required for #current_order to return a new order - # to serve as cart. See https://github.com/spree/spree/blob/1-3-stable/core/lib/spree/core/controller_helpers/order.rb#L14 - # for details. - def expire_current_order - session[:order_id] = nil - @current_order = nil - end - - private - - def payment_method - @payment_method ||= Spree::PaymentMethod.find(params[:payment_method_id]) - end - - def permit_parameters! - params.permit(:token, :payment_method_id, :PayerID) - end - - def reset_order_when_complete - if current_order.complete? - flash[:notice] = t(:order_processed_successfully) - - OrderCompletionReset.new(self, current_order).call - session[:access_token] = current_order.token - end - end - - def reset_to_cart - OrderCheckoutRestart.new(@order).call - handle_insufficient_stock - end - - # See #1074 and #1837 for more detail on why we need this - # An 'orphaned' Spree::Payment is created for every call to CheckoutController#update - # for orders that are processed using a Spree::Gateway::PayPalExpress payment method - # These payments are 'orphaned' because they are never used by the spree_paypal_express gem - # which creates a brand new Spree::Payment from scratch in PayPalController#confirm - # However, the 'orphaned' payments are useful when applying a transaction fee, because the fees - # need to be calculated before the order details are sent to PayPal for confirmation - # This is our best hook for removing the orphaned payments at an appropriate time. ie. after - # the payment details have been confirmed, but before any payments have been processed - def destroy_orphaned_paypal_payments - return unless payment_method.is_a?(Spree::Gateway::PayPalExpress) - - orphaned_payments = current_order.payments.where(payment_method_id: payment_method.id, source_id: nil) - orphaned_payments.each(&:destroy) - end - - def completion_route(order) - spree.order_path(order, token: order.token) - end - - def express_checkout_request_details(order, items) - { - SetExpressCheckoutRequestDetails: { - InvoiceID: order.number, - BuyerEmail: order.email, - ReturnURL: spree.confirm_paypal_url(payment_method_id: params[:payment_method_id], utm_nooverride: 1), - CancelURL: spree.cancel_paypal_url, - SolutionType: payment_method.preferred_solution.presence || "Mark", - LandingPage: payment_method.preferred_landing_page.presence || "Billing", - cppheaderimage: payment_method.preferred_logourl.presence || "", - NoShipping: 1, - PaymentDetails: [payment_details(items)] - } - } - end -end From 2dce66f0790e1123e6f5fad75b63c46efec0508d Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 12:06:56 +0000 Subject: [PATCH 07/23] Fix some simple Rubocop offences --- app/controllers/spree/paypal_controller.rb | 77 +++++++++++----------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/app/controllers/spree/paypal_controller.rb b/app/controllers/spree/paypal_controller.rb index 4d1aaa465a..04cb987bc2 100644 --- a/app/controllers/spree/paypal_controller.rb +++ b/app/controllers/spree/paypal_controller.rb @@ -102,14 +102,14 @@ module Spree 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" + Name: item.product.name, + Number: item.variant.sku, + Quantity: item.quantity, + Amount: { + currencyID: item.order.currency, + value: item.price + }, + ItemCategory: "Physical" } end @@ -118,7 +118,9 @@ module Spree SetExpressCheckoutRequestDetails: { InvoiceID: order.number, BuyerEmail: order.email, - ReturnURL: spree.confirm_paypal_url(payment_method_id: params[:payment_method_id], utm_nooverride: 1), + ReturnURL: spree.confirm_paypal_url( + payment_method_id: params[:payment_method_id], utm_nooverride: 1 + ), CancelURL: spree.cancel_paypal_url, SolutionType: payment_method.preferred_solution.presence || "Mark", LandingPage: payment_method.preferred_landing_page.presence || "Billing", @@ -163,7 +165,8 @@ module Spree def destroy_orphaned_paypal_payments return unless payment_method.is_a?(Spree::Gateway::PayPalExpress) - orphaned_payments = current_order.payments.where(payment_method_id: payment_method.id, source_id: nil) + orphaned_payments = current_order.payments. + where(payment_method_id: payment_method.id, source_id: nil) orphaned_payments.each(&:destroy) end @@ -185,33 +188,33 @@ module Spree # Paypal does not support no items or a zero dollar ItemTotal # This results in the order summary being simply "Current purchase" { - :OrderTotal => { - :currencyID => current_order.currency, - :value => current_order.total + OrderTotal: { + currencyID: current_order.currency, + value: current_order.total } } else { - :OrderTotal => { - :currencyID => current_order.currency, - :value => current_order.total + OrderTotal: { + currencyID: current_order.currency, + value: current_order.total }, - :ItemTotal => { - :currencyID => current_order.currency, - :value => item_sum + ItemTotal: { + currencyID: current_order.currency, + value: item_sum }, - :ShippingTotal => { - :currencyID => current_order.currency, - :value => current_order.ship_total + ShippingTotal: { + currencyID: current_order.currency, + value: current_order.ship_total }, - :TaxTotal => { - :currencyID => current_order.currency, - :value => tax_adjustments_total, + TaxTotal: { + currencyID: current_order.currency, + value: tax_adjustments_total, }, - :ShipToAddress => address_options, - :PaymentDetailsItem => items, - :ShippingMethod => "Shipping Method Name Goes Here", - :PaymentAction => "Sale" + ShipToAddress: address_options, + PaymentDetailsItem: items, + ShippingMethod: "Shipping Method Name Goes Here", + PaymentAction: "Sale" } end end @@ -220,14 +223,14 @@ module Spree return {} unless address_required? { - :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: 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 } end From 2a27da1cc55c9714618f3f8fd9033b2545351f9c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 12:08:39 +0000 Subject: [PATCH 08/23] Bring in Spree::Gateway::PayPalExpress --- app/models/spree/gateway/pay_pal_express.rb | 106 ++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 app/models/spree/gateway/pay_pal_express.rb diff --git a/app/models/spree/gateway/pay_pal_express.rb b/app/models/spree/gateway/pay_pal_express.rb new file mode 100644 index 0000000000..0cb4d38150 --- /dev/null +++ b/app/models/spree/gateway/pay_pal_express.rb @@ -0,0 +1,106 @@ +require 'paypal-sdk-merchant' +module Spree + class Gateway::PayPalExpress < Gateway + preference :login, :string + preference :password, :password + preference :signature, :string + preference :server, :string, default: 'sandbox' + preference :solution, :string, default: 'Mark' + preference :landing_page, :string, default: 'Billing' + preference :logourl, :string, default: '' + + def supports?(source) + true + end + + def provider_class + ::PayPal::SDK::Merchant::API + end + + def provider + ::PayPal::SDK.configure( + :mode => preferred_server.present? ? preferred_server : "sandbox", + :username => preferred_login, + :password => preferred_password, + :signature => preferred_signature) + provider_class.new + end + + def auto_capture? + true + end + + def method_type + 'paypal' + end + + def purchase(amount, express_checkout, gateway_options={}) + pp_details_request = provider.build_get_express_checkout_details({ + :Token => express_checkout.token + }) + pp_details_response = provider.get_express_checkout_details(pp_details_request) + + pp_request = provider.build_do_express_checkout_payment({ + :DoExpressCheckoutPaymentRequestDetails => { + :PaymentAction => "Sale", + :Token => express_checkout.token, + :PayerID => express_checkout.payer_id, + :PaymentDetails => pp_details_response.get_express_checkout_details_response_details.PaymentDetails + } + }) + + pp_response = provider.do_express_checkout_payment(pp_request) + if pp_response.success? + # We need to store the transaction id for the future. + # 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 + else + class << pp_response + def to_s + errors.map(&:long_message).join(" ") + end + end + pp_response + end + end + + def refund(payment, amount) + refund_type = payment.amount == amount.to_f ? "Full" : "Partial" + refund_transaction = provider.build_refund_transaction({ + :TransactionID => payment.source.transaction_id, + :RefundType => refund_type, + :Amount => { + :currencyID => payment.currency, + :value => amount }, + :RefundSource => "any" }) + refund_transaction_response = provider.refund_transaction(refund_transaction) + if refund_transaction_response.success? + payment.source.update_attributes({ + :refunded_at => Time.now, + :refund_transaction_id => refund_transaction_response.RefundTransactionID, + :state => "refunded", + :refund_type => refund_type + } ) + + payment.class.create!( + :order => payment.order, + :source => payment, + :payment_method => payment.payment_method, + :amount => amount.to_f.abs * -1, + :response_code => refund_transaction_response.RefundTransactionID, + :state => 'completed' + ) + end + refund_transaction_response + end + end +end + +# payment.state = 'completed' +# current_order.state = 'complete' From 7f8fe631dd223ade35d39613ec231c101e0b64f0 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 12:10:22 +0000 Subject: [PATCH 09/23] Bring in Spree::PaypalExpressCheckout --- app/models/spree/paypal_express_checkout.rb | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 app/models/spree/paypal_express_checkout.rb diff --git a/app/models/spree/paypal_express_checkout.rb b/app/models/spree/paypal_express_checkout.rb new file mode 100644 index 0000000000..93e0b0e6d0 --- /dev/null +++ b/app/models/spree/paypal_express_checkout.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +module Spree + class PaypalExpressCheckout < ActiveRecord::Base + end +end From 5c7dc6621b46d38b30b72d21f41a926564783be1 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 12:25:10 +0000 Subject: [PATCH 10/23] Bring in views (and convert from ERB to HAML) --- .../admin/payments/_paypal_complete.html.haml | 18 ++++++++++ .../admin/payments/paypal_refund.html.haml | 26 ++++++++++++++ .../payments/source_forms/_paypal.html.haml | 6 ---- .../payments/source_views/_paypal.html.haml | 34 +++++++++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 app/views/spree/admin/payments/_paypal_complete.html.haml create mode 100644 app/views/spree/admin/payments/paypal_refund.html.haml create mode 100644 app/views/spree/admin/payments/source_views/_paypal.html.haml diff --git a/app/views/spree/admin/payments/_paypal_complete.html.haml b/app/views/spree/admin/payments/_paypal_complete.html.haml new file mode 100644 index 0000000000..31addc1a12 --- /dev/null +++ b/app/views/spree/admin/payments/_paypal_complete.html.haml @@ -0,0 +1,18 @@ += form_tag paypal_refund_admin_order_payment_path(@order, @payment) do + .label-block.left.five.columns.alpha + %div + %fieldset{"data-hook" => "admin_variant_new_form"} + %legend= Spree.t('refund', :scope => :paypal) + .field + = label_tag 'refund_amount', Spree.t(:refund_amount, :scope => 'paypal') + %small + %em= Spree.t(:original_amount, :scope => 'paypal', :amount => @payment.display_amount) + %br/ + - symbol = ::Money.new(1, Spree::Config[:currency]).symbol + - if Spree::Config[:currency_symbol_position] == "before" + = symbol + = text_field_tag 'refund_amount', @payment.amount + - else + = text_field_tag 'refund_amount', @payment.amount + = symbol + = button Spree.t(:refund, :scope => 'paypal'), 'icon-dollar' diff --git a/app/views/spree/admin/payments/paypal_refund.html.haml b/app/views/spree/admin/payments/paypal_refund.html.haml new file mode 100644 index 0000000000..d9855483bf --- /dev/null +++ b/app/views/spree/admin/payments/paypal_refund.html.haml @@ -0,0 +1,26 @@ += render :partial => 'spree/admin/shared/order_tabs', :locals => { :current => 'Payments' } +- content_for :page_title do + %i.icon-arrow-right + = link_to Spree.t(:payments), admin_order_payments_path(@order) + %i.icon-arrow-right + = payment_method_name(@payment) + %i.icon-arrow-right + = Spree.t('refund', :scope => :paypal) += form_tag paypal_refund_admin_order_payment_path(@order, @payment) do + .label-block.left.five.columns.alpha + %div + %fieldset{"data-hook" => "admin_variant_new_form"} + %legend= Spree.t('refund', :scope => :paypal) + .field + = label_tag 'refund_amount', Spree.t(:refund_amount, :scope => 'paypal') + %small + %em= Spree.t(:original_amount, :scope => 'paypal', :amount => @payment.display_amount) + %br/ + - symbol = ::Money.new(1, Spree::Config[:currency]).symbol + - if Spree::Config[:currency_symbol_position] == "before" + = symbol + = text_field_tag 'refund_amount', @payment.amount + - else + = text_field_tag 'refund_amount', @payment.amount + = symbol + = button Spree.t(:refund, :scope => 'paypal'), 'icon-dollar' diff --git a/app/views/spree/admin/payments/source_forms/_paypal.html.haml b/app/views/spree/admin/payments/source_forms/_paypal.html.haml index 124139dae0..dbdfe44d7a 100644 --- a/app/views/spree/admin/payments/source_forms/_paypal.html.haml +++ b/app/views/spree/admin/payments/source_forms/_paypal.html.haml @@ -1,8 +1,2 @@ --# We can remove this file as soon as we have a version of better_spree_paypal_express that includes: --# https://github.com/spree-contrib/better_spree_paypal_express/commit/4360a1fb82d30d7601bc6a98e7b74819f0b377f0 - --# The selectors in app/assets/javascripts/spree/backend/paypal_express.js don't work with the version --# of the views we are using, so the warning below wasn't displaying without this override. - #paypal-warning %strong= t('.no_payment_via_admin_backend', :scope => 'paypal') diff --git a/app/views/spree/admin/payments/source_views/_paypal.html.haml b/app/views/spree/admin/payments/source_views/_paypal.html.haml new file mode 100644 index 0000000000..f6bfa4a3cc --- /dev/null +++ b/app/views/spree/admin/payments/source_views/_paypal.html.haml @@ -0,0 +1,34 @@ +%fieldset{"data-hook" => "paypal"} + %legend{align: "center"}= Spree.t(:transaction, :scope => :paypal) + .row + .alpha.six.columns + %dl + %dt + = Spree.t(:payer_id, :scope => :paypal) + \: + %dd= payment.source.payer_id + %dt + = Spree.t(:token, :scope => :paypal) + \: + %dd= payment.source.token + %dt + = Spree.t(:transaction_id) + \: + %dd= payment.source.transaction_id + - if payment.source.state != 'refunded' + = button_link_to Spree.t('actions.refund', :scope => :paypal), spree.paypal_refund_admin_order_payment_path(@order, payment), :icon => 'icon-dollar' + - else + .alpha.six.columns + %dl + %dt + = Spree.t(:state, :scope => :paypal) + \: + %dd= payment.source.state.titleize + %dt + = Spree.t(:refunded_at, :scope => :paypal) + \: + %dd= pretty_time(payment.source.refunded_at) + %dt + = Spree.t(:refund_transaction_id, :scope => :paypal) + \: + %dd= payment.source.refund_transaction_id From ab43c04ca82233a221ede986c2e1da56b9470066 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 12:28:24 +0000 Subject: [PATCH 11/23] Bring in translations --- config/locales/en.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/config/locales/en.yml b/config/locales/en.yml index 613f2adb45..3a4700a9f5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3679,6 +3679,24 @@ See the %{link} to find out more about %{sitename}'s features and to start using ended: ended paused: paused canceled: cancelled + paypal: + already_refunded: "This payment has been refunded and no further action can be taken on it." + no_payment_via_admin_backend: "You cannot charge PayPal accounts through the admin backend at this time." + transaction: "PayPal Transaction" + payer_id: "Payer ID" + transaction_id: "Transaction ID" + token: "Token" + refund: "Refund" + refund_amount: "Amount" + original_amount: "Original amount: %{amount}" + refund_successful: "PayPal refund successful" + refund_unsuccessful: "PayPal refund unsuccessful" + actions: + refund: "Refund" + flash: + cancel: "Don't want to use PayPal? No problems." + connection_failed: "Could not connect to PayPal." + generic_error: "PayPal failed. %{reasons}" users: form: account_settings: Account Settings From 2864f62a08dc25b27132fb49447ed5fe36f5bf67 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 13:14:28 +0000 Subject: [PATCH 12/23] Fix missing semicolons in spree_paypal_express.js --- app/assets/javascripts/admin/spree_paypal_express.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/spree_paypal_express.js b/app/assets/javascripts/admin/spree_paypal_express.js index 7bdf9b187e..26e6ccc630 100644 --- a/app/assets/javascripts/admin/spree_paypal_express.js +++ b/app/assets/javascripts/admin/spree_paypal_express.js @@ -10,7 +10,7 @@ SpreePaypalExpress = { } else if (SpreePaypalExpress.paymentMethodID) { $('.payment-method-settings').children().show(); $('button[type=submit]').prop('disabled', ''); - $('#payment_amount').prop('disabled', '') + $('#payment_amount').prop('disabled', ''); $('#paypal-warning').hide(); } } @@ -22,4 +22,4 @@ $(document).ready(function() { paymentMethods = $('[data-hook="payment_method_field"] input[type="radio"]').click(function (e) { SpreePaypalExpress.hideSettings($(e.target)); }); -}) +}); From 618738db692940a03cb6ae79d678bef7f480545d Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 13:12:46 +0000 Subject: [PATCH 13/23] Fix more Rubocop offences --- app/controllers/spree/paypal_controller.rb | 28 +-- app/models/spree/gateway/pay_pal_express.rb | 198 ++++++++++---------- 2 files changed, 118 insertions(+), 108 deletions(-) diff --git a/app/controllers/spree/paypal_controller.rb b/app/controllers/spree/paypal_controller.rb index 04cb987bc2..0b44ce3763 100644 --- a/app/controllers/spree/paypal_controller.rb +++ b/app/controllers/spree/paypal_controller.rb @@ -40,7 +40,10 @@ module Spree items.reject! do |item| item[:Amount][:value].zero? end - pp_request = provider.build_set_express_checkout(express_checkout_request_details(order, items)) + + pp_request = provider.build_set_express_checkout( + express_checkout_request_details(order, items) + ) begin pp_response = provider.set_express_checkout(pp_request) @@ -66,14 +69,14 @@ module Spree # last chance to interact with the payment process before the money moves... return reset_to_cart unless sufficient_stock? - @order.payments.create!({ - source: Spree::PaypalExpressCheckout.create({ + @order.payments.create!( + source: Spree::PaypalExpressCheckout.create( token: params[:token], payer_id: params[:PayerID] - }), + ), amount: @order.total, payment_method: payment_method - }) + ) @order.next if @order.complete? flash.notice = Spree.t(:order_processed_successfully) @@ -106,8 +109,8 @@ module Spree Number: item.variant.sku, Quantity: item.quantity, Amount: { - currencyID: item.order.currency, - value: item.price + currencyID: item.order.currency, + value: item.price }, ItemCategory: "Physical" } @@ -140,12 +143,11 @@ module Spree end def reset_order_when_complete - if current_order.complete? - flash[:notice] = t(:order_processed_successfully) + return unless current_order.complete? - OrderCompletionReset.new(self, current_order).call - session[:access_token] = current_order.token - end + flash[:notice] = t(:order_processed_successfully) + OrderCompletionReset.new(self, current_order).call + session[:access_token] = current_order.token end def reset_to_cart @@ -174,7 +176,7 @@ module Spree payment_method.provider end - def payment_details items + def payment_details(items) item_sum = items.sum { |i| i[:Quantity] * i[:Amount][:value] } # Would use tax_total here, but it can include "included" taxes as well. # For instance, tax_total would include the 10% GST in Australian stores. diff --git a/app/models/spree/gateway/pay_pal_express.rb b/app/models/spree/gateway/pay_pal_express.rb index 0cb4d38150..357d563264 100644 --- a/app/models/spree/gateway/pay_pal_express.rb +++ b/app/models/spree/gateway/pay_pal_express.rb @@ -1,106 +1,114 @@ +# frozen_string_literal: true + require 'paypal-sdk-merchant' + module Spree - class Gateway::PayPalExpress < Gateway - preference :login, :string - preference :password, :password - preference :signature, :string - preference :server, :string, default: 'sandbox' - preference :solution, :string, default: 'Mark' - preference :landing_page, :string, default: 'Billing' - preference :logourl, :string, default: '' + class Gateway + class PayPalExpress < Gateway + preference :login, :string + preference :password, :password + preference :signature, :string + preference :server, :string, default: 'sandbox' + preference :solution, :string, default: 'Mark' + preference :landing_page, :string, default: 'Billing' + preference :logourl, :string, default: '' - def supports?(source) - true - end - - def provider_class - ::PayPal::SDK::Merchant::API - end - - def provider - ::PayPal::SDK.configure( - :mode => preferred_server.present? ? preferred_server : "sandbox", - :username => preferred_login, - :password => preferred_password, - :signature => preferred_signature) - provider_class.new - end - - def auto_capture? - true - end - - def method_type - 'paypal' - end - - def purchase(amount, express_checkout, gateway_options={}) - pp_details_request = provider.build_get_express_checkout_details({ - :Token => express_checkout.token - }) - pp_details_response = provider.get_express_checkout_details(pp_details_request) - - pp_request = provider.build_do_express_checkout_payment({ - :DoExpressCheckoutPaymentRequestDetails => { - :PaymentAction => "Sale", - :Token => express_checkout.token, - :PayerID => express_checkout.payer_id, - :PaymentDetails => pp_details_response.get_express_checkout_details_response_details.PaymentDetails - } - }) - - pp_response = provider.do_express_checkout_payment(pp_request) - if pp_response.success? - # We need to store the transaction id for the future. - # 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 - else - class << pp_response - def to_s - errors.map(&:long_message).join(" ") - end - end - pp_response + def supports?(_source) + true end - end - def refund(payment, amount) - refund_type = payment.amount == amount.to_f ? "Full" : "Partial" - refund_transaction = provider.build_refund_transaction({ - :TransactionID => payment.source.transaction_id, - :RefundType => refund_type, - :Amount => { - :currencyID => payment.currency, - :value => amount }, - :RefundSource => "any" }) - refund_transaction_response = provider.refund_transaction(refund_transaction) - if refund_transaction_response.success? - payment.source.update_attributes({ - :refunded_at => Time.now, - :refund_transaction_id => refund_transaction_response.RefundTransactionID, - :state => "refunded", - :refund_type => refund_type - } ) + def provider_class + ::PayPal::SDK::Merchant::API + end - payment.class.create!( - :order => payment.order, - :source => payment, - :payment_method => payment.payment_method, - :amount => amount.to_f.abs * -1, - :response_code => refund_transaction_response.RefundTransactionID, - :state => 'completed' + def provider + ::PayPal::SDK.configure( + mode: preferred_server.presence || "sandbox", + username: preferred_login, + password: preferred_password, + signature: preferred_signature ) + provider_class.new + end + + def auto_capture? + true + end + + def method_type + 'paypal' + end + + def purchase(_amount, express_checkout, _gateway_options = {}) + pp_details_request = provider.build_get_express_checkout_details( + Token: express_checkout.token + ) + pp_details_response = provider.get_express_checkout_details(pp_details_request) + + pp_request = provider.build_do_express_checkout_payment( + DoExpressCheckoutPaymentRequestDetails: { + PaymentAction: "Sale", + Token: express_checkout.token, + PayerID: express_checkout.payer_id, + PaymentDetails: pp_details_response. + get_express_checkout_details_response_details.PaymentDetails + } + ) + + pp_response = provider.do_express_checkout_payment(pp_request) + if pp_response.success? + # We need to store the transaction id for the future. + # 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 + else + class << pp_response + def to_s + errors.map(&:long_message).join(" ") + end + end + pp_response + end + end + + def refund(payment, amount) + refund_type = payment.amount == amount.to_f ? "Full" : "Partial" + refund_transaction = provider.build_refund_transaction( + TransactionID: payment.source.transaction_id, + RefundType: refund_type, + Amount: { + currencyID: payment.currency, + value: amount + }, + RefundSource: "any" + ) + refund_transaction_response = provider.refund_transaction(refund_transaction) + if refund_transaction_response.success? + payment.source.update_attributes( + refunded_at: Time.now, + refund_transaction_id: refund_transaction_response.RefundTransactionID, + state: "refunded", + refund_type: refund_type + ) + + payment.class.create!( + order: payment.order, + source: payment, + payment_method: payment.payment_method, + amount: amount.to_f.abs * -1, + response_code: refund_transaction_response.RefundTransactionID, + state: 'completed' + ) + end + refund_transaction_response end - refund_transaction_response end end end - -# payment.state = 'completed' -# current_order.state = 'complete' From 3487898f68e5669a1b4348d760c1ab8410550d86 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 13:33:48 +0000 Subject: [PATCH 14/23] Remove better_spree_paypal_express gem --- Gemfile | 6 ------ Gemfile.lock | 14 -------------- 2 files changed, 20 deletions(-) diff --git a/Gemfile b/Gemfile index 087f734e45..19937fb6b9 100644 --- a/Gemfile +++ b/Gemfile @@ -33,12 +33,6 @@ gem 'ransack', '~> 1.8.10' gem 'state_machines-activerecord' gem 'stringex', '~> 2.8.5' -# Our branch contains the following changes: -# - Pass customer email and phone number to PayPal (merged to upstream master) -# - Change type of password from string to password to hide it in the form -# - Skip CA cert file and use the ones provided by the OS -gem 'spree_paypal_express', github: 'openfoodfoundation/better_spree_paypal_express', branch: '2-1-0-stable' - gem 'stripe' # We need at least this version to have Digicert's root certificate diff --git a/Gemfile.lock b/Gemfile.lock index bc70a93fd6..3e8d745f0a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,14 +4,6 @@ GIT specs: custom_error_message (1.1.1) -GIT - remote: https://github.com/openfoodfoundation/better_spree_paypal_express.git - revision: 1a477e9f7763297944cc99b6f4dd3d962aa963e9 - branch: 2-1-0-stable - specs: - spree_paypal_express (2.0.3) - paypal-sdk-merchant (= 1.106.1) - GIT remote: https://github.com/openfoodfoundation/ofn-qz.git revision: 467f6ea1c44529c7c91cac4c8211bbd863588c0b @@ -490,11 +482,6 @@ GEM activerecord (>= 4.0, < 6.2) parser (3.0.0.0) ast (~> 2.4.1) - paypal-sdk-core (0.2.10) - multi_json (~> 1.0) - xml-simple - paypal-sdk-merchant (1.106.1) - paypal-sdk-core (~> 0.2.3) pg (0.21.0) power_assert (1.2.0) pry (0.13.1) @@ -822,7 +809,6 @@ DEPENDENCIES selenium-webdriver shoulda-matchers simplecov - spree_paypal_express! spring spring-commands-rspec state_machines-activerecord From c34ae0af4ba417574846e2a5643377356a98668a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 13:38:24 +0000 Subject: [PATCH 15/23] Bring in paypal-sdk-merchant gem dependency --- Gemfile | 1 + Gemfile.lock | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/Gemfile b/Gemfile index 19937fb6b9..44cb72d5d3 100644 --- a/Gemfile +++ b/Gemfile @@ -33,6 +33,7 @@ gem 'ransack', '~> 1.8.10' gem 'state_machines-activerecord' gem 'stringex', '~> 2.8.5' +gem 'paypal-sdk-merchant', '1.106.1' gem 'stripe' # We need at least this version to have Digicert's root certificate diff --git a/Gemfile.lock b/Gemfile.lock index 3e8d745f0a..e1f5775524 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -482,6 +482,11 @@ GEM activerecord (>= 4.0, < 6.2) parser (3.0.0.0) ast (~> 2.4.1) + paypal-sdk-core (0.2.10) + multi_json (~> 1.0) + xml-simple + paypal-sdk-merchant (1.106.1) + paypal-sdk-core (~> 0.2.3) pg (0.21.0) power_assert (1.2.0) pry (0.13.1) @@ -784,6 +789,7 @@ DEPENDENCIES paper_trail (~> 7.1.3) paperclip (~> 3.4.1) paranoia (~> 2.4) + paypal-sdk-merchant (= 1.106.1) pg (~> 0.21.0) pry pry-byebug From 83f58368c7c6ad5e9e841c7955e41725dafa5e3c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 13:47:48 +0000 Subject: [PATCH 16/23] Fix class-loading issue in test suite Fixes: ``` Failure/Error: include Spree::Core::ControllerHelpers::Auth NameError: uninitialized constant Spree::Core::ControllerHelpers::Auth # ./lib/spree/api/controller_setup.rb:19:in `block in included' # ./lib/spree/api/controller_setup.rb:5:in `class_eval' # ./lib/spree/api/controller_setup.rb:5:in `included' # ./app/controllers/api/base_controller.rb:9:in `include' # ./app/controllers/api/base_controller.rb:9:in `' # ./app/controllers/api/base_controller.rb:6:in `' # ./app/controllers/api/base_controller.rb:5:in `' # ./app/controllers/api/products_controller.rb:5:in `' # ./app/controllers/api/products_controller.rb:4:in `' # ./spec/controllers/api/products_controller_spec.rb:6:in `' ``` --- lib/spree/api/controller_setup.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/spree/api/controller_setup.rb b/lib/spree/api/controller_setup.rb index 7d9215a332..3371677563 100644 --- a/lib/spree/api/controller_setup.rb +++ b/lib/spree/api/controller_setup.rb @@ -1,3 +1,5 @@ +require 'spree/core/controller_helpers/auth' + module Spree module Api module ControllerSetup From bf47db17922f533da795ff0436c5a16fb16db6d4 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 29 Dec 2020 15:32:52 +0000 Subject: [PATCH 17/23] Fix missing route in Spree::OrdersController I'm not sure why this spec started failing. Fixes: ``` Spree::OrdersController viewing cart when an item is in the cart the page provides the right registration path Failure/Error: expect(subject.registration_path).to eq registration_path ActionController::UrlGenerationError: No route matches {:action=>"index", :controller=>"registration"} # ./spec/controllers/spree/orders_controller_spec.rb:140:in `block (5 levels) in ' ``` --- app/controllers/spree/orders_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb index 740f563bf5..616300c2b1 100644 --- a/app/controllers/spree/orders_controller.rb +++ b/app/controllers/spree/orders_controller.rb @@ -1,6 +1,8 @@ module Spree class OrdersController < Spree::StoreController include OrderCyclesHelper + include Rails.application.routes.url_helpers + layout 'darkswarm' ssl_required :show From 2e5810d64de0deb3eb2011b634de527254e61f75 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 30 Dec 2020 11:36:36 +0000 Subject: [PATCH 18/23] Fix more Rubocop offences --- .../spree/admin/payments/paypal_refund.html.haml | 14 ++++++++------ .../payments/source_views/_paypal.html.haml | 16 +++++++++------- lib/spree/api/controller_setup.rb | 2 ++ 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/app/views/spree/admin/payments/paypal_refund.html.haml b/app/views/spree/admin/payments/paypal_refund.html.haml index d9855483bf..38f308fdaa 100644 --- a/app/views/spree/admin/payments/paypal_refund.html.haml +++ b/app/views/spree/admin/payments/paypal_refund.html.haml @@ -1,20 +1,22 @@ -= render :partial => 'spree/admin/shared/order_tabs', :locals => { :current => 'Payments' } += render partial: 'spree/admin/shared/order_tabs', locals: { current: 'Payments' } + - content_for :page_title do %i.icon-arrow-right = link_to Spree.t(:payments), admin_order_payments_path(@order) %i.icon-arrow-right = payment_method_name(@payment) %i.icon-arrow-right - = Spree.t('refund', :scope => :paypal) + = Spree.t('refund', scope: :paypal) + = form_tag paypal_refund_admin_order_payment_path(@order, @payment) do .label-block.left.five.columns.alpha %div %fieldset{"data-hook" => "admin_variant_new_form"} - %legend= Spree.t('refund', :scope => :paypal) + %legend= Spree.t('refund', scope: :paypal) .field - = label_tag 'refund_amount', Spree.t(:refund_amount, :scope => 'paypal') + = label_tag 'refund_amount', Spree.t(:refund_amount, scope: 'paypal') %small - %em= Spree.t(:original_amount, :scope => 'paypal', :amount => @payment.display_amount) + %em= Spree.t(:original_amount, scope: 'paypal', amount: @payment.display_amount) %br/ - symbol = ::Money.new(1, Spree::Config[:currency]).symbol - if Spree::Config[:currency_symbol_position] == "before" @@ -23,4 +25,4 @@ - else = text_field_tag 'refund_amount', @payment.amount = symbol - = button Spree.t(:refund, :scope => 'paypal'), 'icon-dollar' + = button Spree.t(:refund, scope: 'paypal'), 'icon-dollar' diff --git a/app/views/spree/admin/payments/source_views/_paypal.html.haml b/app/views/spree/admin/payments/source_views/_paypal.html.haml index f6bfa4a3cc..4636995466 100644 --- a/app/views/spree/admin/payments/source_views/_paypal.html.haml +++ b/app/views/spree/admin/payments/source_views/_paypal.html.haml @@ -1,14 +1,14 @@ %fieldset{"data-hook" => "paypal"} - %legend{align: "center"}= Spree.t(:transaction, :scope => :paypal) + %legend{align: "center"}= Spree.t(:transaction, scope: :paypal) .row .alpha.six.columns %dl %dt - = Spree.t(:payer_id, :scope => :paypal) + = Spree.t(:payer_id, scope: :paypal) \: %dd= payment.source.payer_id %dt - = Spree.t(:token, :scope => :paypal) + = Spree.t(:token, scope: :paypal) \: %dd= payment.source.token %dt @@ -16,19 +16,21 @@ \: %dd= payment.source.transaction_id - if payment.source.state != 'refunded' - = button_link_to Spree.t('actions.refund', :scope => :paypal), spree.paypal_refund_admin_order_payment_path(@order, payment), :icon => 'icon-dollar' + = button_link_to Spree.t('actions.refund', scope: :paypal), + spree.paypal_refund_admin_order_payment_path(@order, payment), + icon: 'icon-dollar' - else .alpha.six.columns %dl %dt - = Spree.t(:state, :scope => :paypal) + = Spree.t(:state, scope: :paypal) \: %dd= payment.source.state.titleize %dt - = Spree.t(:refunded_at, :scope => :paypal) + = Spree.t(:refunded_at, scope: :paypal) \: %dd= pretty_time(payment.source.refunded_at) %dt - = Spree.t(:refund_transaction_id, :scope => :paypal) + = Spree.t(:refund_transaction_id, scope: :paypal) \: %dd= payment.source.refund_transaction_id diff --git a/lib/spree/api/controller_setup.rb b/lib/spree/api/controller_setup.rb index 3371677563..010ce77b1e 100644 --- a/lib/spree/api/controller_setup.rb +++ b/lib/spree/api/controller_setup.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spree/core/controller_helpers/auth' module Spree From 09b7512cd8196f643440448d9f885e3996670f56 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 5 Jan 2021 11:29:34 +0000 Subject: [PATCH 19/23] Remove dead code around unused payment_method_field data-hook --- app/assets/javascripts/admin/spree_paypal_express.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/assets/javascripts/admin/spree_paypal_express.js b/app/assets/javascripts/admin/spree_paypal_express.js index 26e6ccc630..81d6a899af 100644 --- a/app/assets/javascripts/admin/spree_paypal_express.js +++ b/app/assets/javascripts/admin/spree_paypal_express.js @@ -15,11 +15,3 @@ SpreePaypalExpress = { } } } - -$(document).ready(function() { - checkedPaymentMethod = $('[data-hook="payment_method_field"] input[type="radio"]:checked'); - SpreePaypalExpress.hideSettings(checkedPaymentMethod); - paymentMethods = $('[data-hook="payment_method_field"] input[type="radio"]').click(function (e) { - SpreePaypalExpress.hideSettings($(e.target)); - }); -}); From 8439b68b36fa7215ce838ce8f4621304967d1569 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 5 Jan 2021 11:29:56 +0000 Subject: [PATCH 20/23] Update code comment and link --- app/controllers/spree/paypal_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/spree/paypal_controller.rb b/app/controllers/spree/paypal_controller.rb index 0b44ce3763..1be743cf42 100644 --- a/app/controllers/spree/paypal_controller.rb +++ b/app/controllers/spree/paypal_controller.rb @@ -34,8 +34,7 @@ module Spree end # Because PayPal doesn't accept $0 items at all. - # See #10 - # https://cms.paypal.com/uk/cgi-bin/?cmd=_render-content&content_ID=developer/e_howto_api_ECCustomizing + # 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? From c07a4fcb5e25972292cbb90dba6cab47dbb9b46f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 5 Jan 2021 11:31:45 +0000 Subject: [PATCH 21/23] Remove data-hooks from views --- app/views/spree/admin/payments/_paypal_complete.html.haml | 2 +- app/views/spree/admin/payments/paypal_refund.html.haml | 2 +- app/views/spree/admin/payments/source_views/_paypal.html.haml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/spree/admin/payments/_paypal_complete.html.haml b/app/views/spree/admin/payments/_paypal_complete.html.haml index 31addc1a12..d2ea6405f0 100644 --- a/app/views/spree/admin/payments/_paypal_complete.html.haml +++ b/app/views/spree/admin/payments/_paypal_complete.html.haml @@ -1,7 +1,7 @@ = form_tag paypal_refund_admin_order_payment_path(@order, @payment) do .label-block.left.five.columns.alpha %div - %fieldset{"data-hook" => "admin_variant_new_form"} + %fieldset %legend= Spree.t('refund', :scope => :paypal) .field = label_tag 'refund_amount', Spree.t(:refund_amount, :scope => 'paypal') diff --git a/app/views/spree/admin/payments/paypal_refund.html.haml b/app/views/spree/admin/payments/paypal_refund.html.haml index 38f308fdaa..98641d05a2 100644 --- a/app/views/spree/admin/payments/paypal_refund.html.haml +++ b/app/views/spree/admin/payments/paypal_refund.html.haml @@ -11,7 +11,7 @@ = form_tag paypal_refund_admin_order_payment_path(@order, @payment) do .label-block.left.five.columns.alpha %div - %fieldset{"data-hook" => "admin_variant_new_form"} + %fieldset %legend= Spree.t('refund', scope: :paypal) .field = label_tag 'refund_amount', Spree.t(:refund_amount, scope: 'paypal') diff --git a/app/views/spree/admin/payments/source_views/_paypal.html.haml b/app/views/spree/admin/payments/source_views/_paypal.html.haml index 4636995466..d1094fa000 100644 --- a/app/views/spree/admin/payments/source_views/_paypal.html.haml +++ b/app/views/spree/admin/payments/source_views/_paypal.html.haml @@ -1,4 +1,4 @@ -%fieldset{"data-hook" => "paypal"} +%fieldset %legend{align: "center"}= Spree.t(:transaction, scope: :paypal) .row .alpha.six.columns From f9f830e0e12665ba35ef623c9987a2a2e5fb315e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 6 Jan 2021 15:35:20 +0000 Subject: [PATCH 22/23] Bring in Paypal certificates hack via new initializer --- config/initializers/paypal.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 config/initializers/paypal.rb diff --git a/config/initializers/paypal.rb b/config/initializers/paypal.rb new file mode 100644 index 0000000000..31ec5f826a --- /dev/null +++ b/config/initializers/paypal.rb @@ -0,0 +1,15 @@ +# Fixes the issue about some PayPal requests failing with +# OpenSSL::SSL::SSLError (SSL_connect returned=1 errno=0 state=error: certificate verify failed) +module CAFileHack + # This overrides paypal-sdk-core default so we don't pass the cert the gem provides to the + # NET::HTTP instance. This way we rely on the default behavior of validating the server's cert + # against the CA certs of the OS (we assume), which tend to be up to date. + # + # See https://github.com/openfoodfoundation/openfoodnetwork/issues/5855 for details. + def default_ca_file + nil + end +end + +require 'paypal-sdk-merchant' +PayPal::SDK::Core::Util::HTTPHelper.prepend(CAFileHack) From e5be249f2cc0777140d0c9a0000460d49a349e86 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 6 Jan 2021 15:39:38 +0000 Subject: [PATCH 23/23] Update comment on #expire_current_order --- app/controllers/spree/paypal_controller.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/spree/paypal_controller.rb b/app/controllers/spree/paypal_controller.rb index 1be743cf42..ca44867165 100644 --- a/app/controllers/spree/paypal_controller.rb +++ b/app/controllers/spree/paypal_controller.rb @@ -92,9 +92,7 @@ module Spree redirect_to main_app.checkout_path end - # Clears the cached order. Required for #current_order to return a new order - # to serve as cart. See https://github.com/spree/spree/blob/1-3-stable/core/lib/spree/core/controller_helpers/order.rb#L14 - # for details. + # Clears the cached order. Required for #current_order to return a new order to serve as cart. def expire_current_order session[:order_id] = nil @current_order = nil