shuffle things around on BT error reporting to work with GC

app/controllers/pages_controller.rb

@@ -39,7 +39,8 @@ def show
   end
 
   def follow_up
-    render_liquid(@page.follow_up_liquid_layout, :follow_up)
+    liquid_layout = @page.follow_up_liquid_layout || @page.liquid_layout
+    render_liquid(liquid_layout, :follow_up)
   end
 
   def update

app/controllers/payment_controller.rb

@@ -17,8 +17,11 @@ def transaction
         format.json { render json: { success: true }.merge(id) }
       end
     else
-      errors = client::ErrorProcessing.new(builder.result).process
-      render json: { success: false, errors: errors }, status: 422
+      @errors = client::ErrorProcessing.new(builder.errors).process
+      respond_to do |format|
+        format.html { render :errors }
+        format.json { render json: { success: false, errors: @errors }, status: 422 }
+      end
     end
   end
 

config/routes.rb

@@ -118,7 +118,7 @@
 
   namespace :api do
     namespace :payment do
-      namespace :braintree do
+      namespace :braintree, defaults: {format: 'json'} do
         get 'token'
         post 'pages/:page_id/transaction',  action: 'transaction', as: 'transaction'
         post 'webhook', action: 'webhook'

lib/payment_processor/clients/braintree/error_processing.rb

@@ -2,6 +2,7 @@ module PaymentProcessor
   module Clients
     module Braintree
       class ErrorProcessing
+        attr_reader :errors
 
         # Collection of error codes, generated by braintree for invalid
         # customer/credit card transaction data.
@@ -33,8 +34,8 @@ class ErrorProcessing
 
         FILTER = -> (error) { USER_ERROR_CODES.include?( error.code.to_i ) }
 
-        def initialize(response)
-          @response = response
+        def initialize(errors)
+          @errors = errors
         end
 
         def process
@@ -62,10 +63,6 @@ def system_errors
           @system_errors ||= errors.reject(&FILTER)
         end
 
-        def errors
-          @response.errors
-        end
-
         def processor_errors
           if @response.transaction
             case @response.transaction.status

lib/payment_processor/clients/braintree/subscription.rb

@@ -23,7 +23,7 @@ class Subscription < Populator
         # * +:currency+ - Billing currency (required)
         # * +:user+     - Hash of information describing the customer. Must include email, and name (required)
         # * +:customer+ - Instance of existing Braintree customer. Must respond to +customer_id+ (optional)
-        attr_reader :result, :action
+        attr_reader :action
 
         def self.make_subscription(nonce:, amount:, currency:, user:, page_id:)
           builder = new(nonce, amount, currency, user, page_id)
@@ -53,6 +53,10 @@ def subscription_id
           @result.try(:subscription).try(:id)
         end
 
+        def errors
+          @result.try(:errors)
+        end
+
         private
 
         # if the customer was updated, we have to create the payment method separately,

lib/payment_processor/clients/braintree/transaction.rb

@@ -18,7 +18,7 @@ class Transaction < Populator
         # * +:currency+ - Billing currency (required)
         # * +:user+     - Hash of information describing the customer. Must include email, and name (required)
         # * +:customer+ - Instance of existing Braintree customer. Must respond to +customer_id+ (optional)
-        attr_reader :result, :action
+        attr_reader :action
 
         def self.make_transaction(nonce:, amount:, currency:, user:, page_id:)
           builder = new(nonce, amount, currency, user, page_id)
@@ -37,17 +37,21 @@ def initialize(nonce, amount, currency, user, page_id)
         def transact
           @result = ::Braintree::Transaction.sale(options)
           if @result.success?
-            @action = ManageBraintreeDonation.create(params: @user.merge(page_id: @page_id), braintree_result: result, is_subscription: false)
-            Payment.write_transaction(result, @page_id, @action.member_id, existing_customer)
+            @action = ManageBraintreeDonation.create(params: @user.merge(page_id: @page_id), braintree_result: @result, is_subscription: false)
+            Payment.write_transaction(@result, @page_id, @action.member_id, existing_customer)
           else
-            Payment.write_transaction(result, @page_id, nil, existing_customer)
+            Payment.write_transaction(@result, @page_id, nil, existing_customer)
           end
         end
 
         def transaction_id
           @result.try(:transaction).try(:id)
         end
 
+        def errors
+          @result.try(:errors)
+        end
+
         private
 
         def options

lib/payment_processor/go_cardless/error_processing.rb

@@ -0,0 +1,22 @@
+module PaymentProcessor
+  module GoCardless
+    class ErrorProcessing
+
+      def initialize(errors)
+        @errors = errors
+      end
+
+      def process
+        @errors
+        # return user_errors if user_errors.any?
+        # return processor_errors if processor_errors.any?
+
+        # raise_system_errors if system_errors.any?
+      end
+
+      private
+
+    end
+  end
+end
+

spec/controllers/api/payment/braintree_controller_spec.rb

@@ -41,14 +41,17 @@
         page_id: params[:page_id]
       }
     end
+    before :each do
+      request.accept = "application/json" # ask for json
+    end
+
 
     describe 'successfully' do
 
       describe 'with recurring: true' do
 
         let(:subscription) { instance_double('Braintree::Subscription', id: 's1234')}
-        let(:result) { instance_double('Braintree::SuccessResult', success?: true, subscription: subscription) }
-        let(:builder){ instance_double('PaymentProcessor::Clients::Braintree::Subscription', action: action, result: result) }
+        let(:builder){ instance_double('PaymentProcessor::Clients::Braintree::Subscription', action: action, success?: true, subscription_id: 's1234') }
 
         before do
           allow(client::Subscription).to receive(:make_subscription).and_return(builder)
@@ -75,8 +78,7 @@
       describe 'without recurring' do
 
         let(:transaction) { instance_double('Braintree::Transaction', id: 't1234')}
-        let(:result) { instance_double('Braintree::SuccessResult', success?: true, transaction: transaction) }
-        let(:builder){ instance_double('PaymentProcessor::Clients::Braintree::Transaction', action: action, result: result) }
+        let(:builder){ instance_double('PaymentProcessor::Clients::Braintree::Transaction', action: action, success?: true, transaction_id: 't1234') }
 
         before :each do
           allow(client::Transaction).to receive(:make_transaction).and_return(builder)
@@ -103,7 +105,6 @@
 
     describe 'unsuccessfully' do
 
-      let(:result) { instance_double('Braintree::ErrorResult', success?: false) }
       let(:errors) { instance_double('PaymentProcessor::Clients::Braintree::ErrorProcessing', process: {my_error: 'foo'}) }
 
       before :each do
@@ -112,7 +113,7 @@
 
       describe 'with recurring: true' do
 
-        let(:builder){ instance_double('PaymentProcessor::Clients::Braintree::Subscription', result: result) }
+        let(:builder){ instance_double('PaymentProcessor::Clients::Braintree::Subscription', success?: false, errors: {}) }
 
         before do
           allow(client::Subscription).to receive(:make_subscription).and_return(builder)
@@ -144,7 +145,7 @@
       describe 'without recurring' do
 
         let(:transaction) { instance_double('Braintree::Transaction', id: 't1234')}
-        let(:builder){ instance_double('PaymentProcessor::Clients::Braintree::Transaction', result: result) }
+        let(:builder){ instance_double('PaymentProcessor::Clients::Braintree::Transaction', success?: false, errors: {}) }
 
         before :each do
           allow(client::Transaction).to receive(:make_transaction).and_return(builder)

spec/controllers/pages_controller_spec.rb

@@ -209,14 +209,28 @@
       allow(Page).to receive(:find){ page }
       allow(page).to receive(:update)
       allow(LiquidRenderer).to receive(:new){ renderer }
-      get :follow_up, id: '1'
     end
 
+    subject { get :follow_up, id: '1' }
+
     it 'finds campaign page' do
+      subject
       expect(Page).to have_received(:find).with('1')
     end
 
+    it 'uses main liquid layout if no follow up set' do
+      allow(page).to receive(:follow_up_liquid_layout).and_return(nil)
+      subject
+      expect(LiquidRenderer).to have_received(:new).with(page,
+        location: {},
+        member: nil,
+        layout: page.liquid_layout,
+        url_params: {"id"=>"1", "controller"=>"pages", "action"=>"follow_up"}
+      )
+    end
+
     it 'instantiates a LiquidRenderer and calls render' do
+      subject
       expect(LiquidRenderer).to have_received(:new).with(page,
         location: {},
         member: nil,
@@ -226,43 +240,51 @@
       expect(renderer).to have_received(:render)
     end
 
-    it 'renders show template' do
-      expect(response).to render_template :show
+    it 'renders follow_up template' do
+      subject
+      expect(response).to render_template :follow_up
     end
 
     it 'assigns @data to personalization_data' do
+      subject
       expect(assigns(:data)).to eq(renderer.personalization_data)
     end
 
     it 'assigns campaign' do
+      subject
       expect(assigns(:rendered)).to eq(renderer.render)
     end
 
     it 'redirects to homepage if user not logged in and page unpublished' do
+      subject
       allow(controller).to receive(:user_signed_in?) { false }
       allow(page).to receive(:active?){ false }
       expect( get :follow_up, id: '1' ).to redirect_to(Settings.homepage_url)
     end
 
     it 'does not redirect to homepage if user not logged in and page published' do
+      subject
       allow(controller).to receive(:user_signed_in?) { false }
       allow(page).to receive(:active?){ true }
       expect( get :follow_up, id: '1' ).not_to be_redirect
     end
 
     it 'does not redirect to homepage if user logged in and page unpublished' do
+      subject
       allow(controller).to receive(:user_signed_in?) { true }
       allow(page).to receive(:active?){ false }
       expect( get :follow_up, id: '1' ).not_to be_redirect
     end
 
     it 'does not redirect to homepage if user logged in and page published' do
+      subject
       allow(controller).to receive(:user_signed_in?) { true }
       allow(page).to receive(:active?){ true }
       expect( get :follow_up, id: '1' ).not_to be_redirect
     end
 
     it 'raises 404 if page is not found' do
+      subject
       allow(Page).to receive(:find).and_raise(ActiveRecord::RecordNotFound)
       expect{ get :follow_up, id: '1000000' }.to raise_error(ActiveRecord::RecordNotFound)
     end

spec/requests/api/go_cardless/go_cardless_spec.rb

@@ -175,6 +175,11 @@
       it 'increments action count on Page' do
         expect{ subject }.to change{ page.reload.action_count }.by 1
       end
+
+      it "redirects to the page's follow-up path" do
+        subject
+        expect(response.status).to redirect_to(follow_up_page_path(page))
+      end
     end
 
     describe 'transaction' do
@@ -213,13 +218,6 @@
           expect(form_data).not_to have_key('subscription_id')
         end
 
-        it 'responds successfully with transaction_id' do
-          subject
-          transaction_id = Payment::GoCardless::Transaction.last.go_cardless_id
-          expect(response.status).to eq 200
-          expect(response.body).to eq({ success: true, transaction_id: transaction_id }.to_json)
-        end
-
         it 'creates a Transaction record associated with the Page' do
           expect{ subject }.to change{ Payment::GoCardless::Transaction.count }.by 1
           payment = Payment::GoCardless::Transaction.last
@@ -300,13 +298,6 @@
           expect(form_data).not_to have_key('transaction_id')
         end
 
-        it 'responds successfully with subscription_id' do
-          subject
-          subscription_id = Payment::GoCardless::Subscription.last.go_cardless_id
-          expect(response.status).to eq 200
-          expect(response.body).to eq({ success: true, subscription_id: subscription_id }.to_json)
-        end
-
         it 'does not yet create a transaction record' do
           expect{ subject }.not_to change{ Payment::GoCardless::Transaction.count }
         end