Dry up braintree webhook handling and log more clearly which webhook …

…handling is failing, for which subscription

app/models/payment/go_cardless/transaction.rb

@@ -77,14 +77,13 @@ class Payment::GoCardless::Transaction < ActiveRecord::Base
   end
 
   def publish_failed_subscription_charge
-    return if self.subscription.blank?
+    return if subscription.blank?
     ChampaignQueue.push({
-                          type: 'subscription-payment',
-                          params: {
-                            recurring_id: self.subscription.go_cardless_id,
-                            success: 0
-                          }
-                        }, { delay: 120 })
-
+      type: 'subscription-payment',
+      params: {
+        recurring_id: subscription.go_cardless_id,
+        success: 0
+      }
+    }, { delay: 120 })
   end
 end

lib/payment_processor/braintree/webhook_handler.rb

@@ -51,8 +51,10 @@ def process_notification
         when 'subscription_charged_unsuccessfully'
           handle_failed_subscription_charge
         else
-          Rails.logger.info("Unsupported Braintree::WebhookNotification received of type '#{@notification.kind}'")
+          Rails.logger.info("Unsupported Braintree::WebhookNotification received of type '#{notification.kind}'")
         end
+      rescue
+        Rails.logger.error("Braintree webhook handling failed for '#{notification.kind}', for subscription ID '#{notification.subscription.id}'")
       end
 
       def notification
@@ -69,26 +71,13 @@ def handle_subscription_cancelled
       end
 
       def handle_subscription_charged
-        if original_action.blank?
-          Rails.logger.info("Failed to handle Braintree::WebhookNotification for successful charge on subscription_id '#{@notification.subscription.id}'")
-          return false
-        end
-
-        customer = Payment::Braintree::Customer.find_by(member_id: original_action.member_id)
         record = Payment::Braintree.write_transaction(notification, original_action.page_id, original_action.member_id, customer, false)
         record.update(subscription: subscription)
         record.publish_subscription_charge
-
         true
       end
 
       def handle_failed_subscription_charge
-        if original_action.blank?
-          Rails.logger.info("Failed to handle Braintree::WebhookNotification for failed charge on subscription_id '#{@notification.subscription.id}'")
-          return false
-        end
-
-        customer = Payment::Braintree::Customer.find_by(member_id: original_action.member_id)
         record = Payment::Braintree::Transaction.create!(
           subscription: subscription,
           customer: customer,
@@ -99,7 +88,15 @@ def handle_failed_subscription_charge
 
       # This method should only be called if @notification.subscription is a subscription object
       def original_action
-        @action ||= subscription.try(:action)
+        @action ||= subscription.action
+      rescue ActiveRecord::RecordNotFound
+        Rails.logger.error("No action is associated with Braintree subscription with id #{subscription.id}!")
+      end
+
+      def customer
+        Payment::Braintree::Customer.find_by(member_id: original_action.member_id)
+      rescue ActiveRecord::RecordNotFound
+        Rails.logger.error("No Braintree customer found for member with id #{original_action.member_id}!")
       end
 
       def subscription

lib/payment_processor/go_cardless/webhook_handler/process_events.rb

@@ -28,7 +28,6 @@ def already_processed?(event)
 
       def process_event(event)
         klass_name = event['resource_type'].classify
-
         if ::PaymentProcessor::GoCardless::WebhookHandler.const_defined?(klass_name)
           processor = ::PaymentProcessor::GoCardless::WebhookHandler.const_get(klass_name).new(event)
           processor.process

spec/lib/payment_processor/braintree/webhook_handler_spec.rb

@@ -73,18 +73,14 @@ def notification_faker(type, object_id)
         )
       end
 
-      it 'returns false' do
-        expect(subject).to be false
-      end
-
       it 'does not write transaction' do
         expect { subject }
           .not_to change { subscription.transactions.count }
       end
 
       it 'logs error' do
-        expect(Rails.logger).to receive(:info)
-          .with(/Failed to handle Braintree::WebhookNotification/)
+        expect(Rails.logger).to receive(:error)
+          .with(/Braintree webhook handling failed for 'subscription_charged_successfully', for subscription ID 'invalid_subscription_id'/)
 
         subject
       end

spec/requests/api/braintree/webhook_spec.rb

@@ -123,9 +123,15 @@
             )
           end
 
-          it 'returns not found' do
+          it 'logs an error' do
+            expect(Rails.logger).to receive(:error).with("Braintree webhook handling failed for 'subscription_charged_successfully', for subscription ID 'xxx'")
             subject
-            expect(response.status).to eq 404
+          end
+
+          it 'does not create a transaction' do
+            expect(Payment::Braintree::Transaction.count).to eq(0)
+            subject
+            expect(@subscription.reload.transactions.count).to eq(0)
           end
         end
       end

spec/requests/api/go_cardless/webhooks_spec.rb

@@ -109,19 +109,26 @@
                 'description' => 'Customer cancelled the mandate at their bank branch.',
                 'scheme' => 'bacs',
                 'reason_code' => 'ARRUD-1'
-              },
+              }
             }
           ]
         }.to_json
       end
+      let(:valid) { instance_double(PaymentProcessor::GoCardless::WebhookSignature) }
+
+      before do
+        allow(PaymentProcessor::GoCardless::WebhookSignature).to receive(:new).and_return(valid)
+        allow(valid).to receive(:valid?).and_return(true)
+      end
 
       it 'posts a failed subscription charge to the queue' do
+        expect(ChampaignQueue).to receive(:push).with({
+          type: 'subscription-payment',
+          params: { recurring_id: subscription.go_cardless_id, success: 0 }
+        }, { delay: 120 })
         post('/api/go_cardless/webhook', events, headers)
-        byebug
       end
-
     end
-
   end
 
   describe 'with invalid signature' do