Merge pull request #7099 from Matt-Yorkley/rails-5-fixes

Rails 5.0 fixes

app/controllers/spree/admin/payment_methods_controller.rb

@@ -12,13 +12,11 @@ module Spree
       def create
         force_environment
 
-        @payment_method = params[:payment_method].
-          delete(:type).
-          constantize.
-          new(PermittedAttributes::PaymentMethod.new(params[:payment_method]).call)
+        @payment_method = payment_method_class.constantize.new(base_params)
         @object = @payment_method
 
         invoke_callbacks(:create, :before)
+
         if @payment_method.save
           invoke_callbacks(:create, :after)
           flash[:success] = Spree.t(:successfully_created, resource: Spree.t(:payment_method))
@@ -34,16 +32,16 @@ module Spree
         force_environment
 
         invoke_callbacks(:update, :before)
-        payment_method_type = params[:payment_method].delete(:type)
-        if @payment_method['type'].to_s != payment_method_type
+
+        if @payment_method.type.to_s != payment_method_class
           @payment_method.update_columns(
-            type: payment_method_type,
+            type: payment_method_class,
             updated_at: Time.zone.now
           )
           @payment_method = PaymentMethod.find(params[:id])
         end
 
-        if @payment_method.update(params_for_update)
+        if @payment_method.update(update_params)
           invoke_callbacks(:update, :after)
           flash[:success] = Spree.t(:successfully_updated, resource: Spree.t(:payment_method))
           redirect_to spree.edit_admin_payment_method_path(@payment_method)
@@ -98,8 +96,12 @@ module Spree
 
       private
 
+      def payment_method_class
+        @payment_method_class ||= base_params.delete(:type)
+      end
+
       def force_environment
-        params[:payment_method][:environment] = Rails.env unless spree_current_user.admin?
+        base_params[:environment] = Rails.env unless spree_current_user.admin?
       end
 
       def load_data
@@ -143,7 +145,7 @@ module Spree
         @stripe_account_holder = Enterprise.find(@payment_method.preferred_enterprise_id)
         return if spree_current_user.enterprises.include? @stripe_account_holder
 
-        params[:payment_method][:preferred_enterprise_id] = @stripe_account_holder.id
+        update_params[:preferred_enterprise_id] = @stripe_account_holder.id
       end
 
       def stripe_payment_method?
@@ -155,19 +157,26 @@ module Spree
         provider.name.ends_with?("StripeConnect", "StripeSCA")
       end
 
+      def base_params
+        @base_params ||= PermittedAttributes::PaymentMethod.new(params[:payment_method]).
+          call.to_h.with_indifferent_access
+      end
+
       # Merge payment method params with gateway params like :gateway_stripe_connect
       # Also, remove password if present and blank
-      def params_for_update
-        gateway_params = params[ActiveModel::Naming.param_key(@payment_method)] || {}
-        params_for_update = params[:payment_method].merge(gateway_params)
+      def update_params
+        @update_params ||= begin
+          gateway_params = raw_params[ActiveModel::Naming.param_key(@payment_method)] || {}
+          params_for_update = base_params.merge(gateway_params)
 
-        params_for_update.each do |key, _value|
-          if key.include?("password") && params_for_update[key].blank?
-            params_for_update.delete(key)
+          params_for_update.each do |key, value|
+            if key.include?("password") && value.blank?
+              params_for_update.delete(key)
+            end
           end
-        end
 
-        PermittedAttributes::PaymentMethod.new(params_for_update).call
+          params_for_update
+        end
       end
     end
   end

app/controllers/spree/admin/payments_controller.rb

@@ -3,6 +3,8 @@
 module Spree
   module Admin
     class PaymentsController < Spree::Admin::BaseController
+      include FullUrlHelper
+
       before_action :load_order, except: [:show]
       before_action :load_payment, only: [:fire, :show]
       before_action :load_data
@@ -153,7 +155,7 @@ module Spree
       def authorize_stripe_sca_payment
         return unless @payment.payment_method.class == Spree::Gateway::StripeSCA
 
-        @payment.authorize!(main_app.order_path(@payment.order, only_path: false))
+        @payment.authorize!(full_order_path(@payment.order))
 
         raise Spree::Core::GatewayError, I18n.t('authorization_failure') unless @payment.pending?
 

lib/stripe/authorize_response_patcher.rb

@@ -19,12 +19,15 @@ module Stripe
     private
 
     def url_for_authorization(response)
-      next_action = response.params["next_source_action"]
-      return unless response.params["status"] == "requires_source_action" &&
-                    next_action.present? &&
-                    next_action["type"] == "authorize_with_url"
+      return unless %w(requires_source_action requires_action).include?(response.params["status"])
 
-      url = next_action["authorize_with_url"]["url"]
+      next_action = response.params["next_source_action"] || response.params["next_action"]
+      return unless next_action.present?
+
+      next_action_type = next_action["type"]
+      return unless %w(authorize_with_url redirect_to_url).include?(next_action_type)
+
+      url = next_action[next_action_type]["url"]
       return url if url.match(%r{https?:\/\/[\S]+}) && url.include?("stripe.com")
     end
 

spec/controllers/spree/admin/payment_methods_controller_spec.rb

@@ -52,6 +52,54 @@ module Spree
     end
 
     describe "#update" do
+      context "updating a payment method" do
+        let!(:payment_method) { create(:payment_method, :flat_rate) }
+        let(:params) {
+          {
+            id: payment_method.id,
+            payment_method: {
+              name: "Updated",
+              description: "Updated",
+              type: "Spree::PaymentMethod::Check",
+              calculator_attributes: {
+                id: payment_method.calculator.id,
+                preferred_amount: 456,
+                preferred_currency: "GBP"
+              }
+            }
+          }
+        }
+
+        before { controller_login_as_admin }
+
+        it "updates the payment method" do
+          spree_post :update, params
+          payment_method.reload
+
+          expect(payment_method.name).to eq "Updated"
+          expect(payment_method.description).to eq "Updated"
+          expect(payment_method.calculator.preferred_amount).to eq 456
+          expect(payment_method.calculator.preferred_currency).to eq "GBP"
+        end
+
+        context "when the given payment method type does not match" do
+          let(:params) {
+            {
+              id: payment_method.id,
+              payment_method: {
+                type: "Spree::Gateway::Bogus"
+              }
+            }
+          }
+
+          it "updates the payment method type" do
+            spree_post :update, params
+
+            expect(PaymentMethod.find(payment_method.id).type).to eq "Spree::Gateway::Bogus"
+          end
+        end
+      end
+
       context "on a StripeConnect payment method" do
         let!(:user) { create(:user, enterprise_limit: 2) }
         let!(:enterprise1) { create(:distributor_enterprise, owner: user) }
@@ -61,7 +109,15 @@ module Spree
         before { allow(controller).to receive(:spree_current_user) { user } }
 
         context "when an attempt is made to change the stripe account holder (preferred_enterprise_id)" do
-          let(:params) { { id: payment_method.id, payment_method: { type: "Spree::Gateway::StripeConnect", preferred_enterprise_id: enterprise1.id } } }
+          let(:params) {
+            {
+              id: payment_method.id,
+              payment_method: {
+                type: "Spree::Gateway::StripeConnect",
+                preferred_enterprise_id: enterprise1.id
+              }
+            }
+          }
 
           context "as a user that does not manage the existing stripe account holder" do
             it "prevents the stripe account holder from being updated" do