Deliver email when gocardless payment fails

app/jobs/payment_requests/payments/gocardless_create_job.rb

@@ -9,6 +9,9 @@ class GocardlessCreateJob < ApplicationJob
 
       def perform(payable)
         result = PaymentRequests::Payments::GocardlessService.new(payable).create
+
+        PaymentRequestMailer.with(payment_request: payable).requested.deliver_later if result.payable&.payment_failed?
+
         result.raise_if_error!
       end
     end

app/services/payment_requests/payments/gocardless_service.rb

@@ -41,6 +41,7 @@ def create
         payable.increment_payment_attempts!
 
         gocardless_result = create_gocardless_payment
+        return result unless gocardless_result
 
         payment = Payment.new(
           payable: payable,
@@ -52,13 +53,11 @@ def create
           status: gocardless_result.status
         )
 
-        ActiveRecord::Base.transaction do
-          payment.save!
+        payment.save!
 
-          payable_payment_status = payable_payment_status(payment.status)
-          update_payable_payment_status(payment_status: payable_payment_status)
-          update_invoices_payment_status(payment_status: payable_payment_status)
-        end
+        payable_payment_status = payable_payment_status(payment.status)
+        update_payable_payment_status(payment_status: payable_payment_status)
+        update_invoices_payment_status(payment_status: payable_payment_status)
 
         Integrations::Aggregator::Payments::CreateJob.perform_later(payment:) if payment.should_sync_payment?
 
@@ -155,7 +154,8 @@ def create_gocardless_payment
         deliver_error_webhook(e)
         update_payable_payment_status(payment_status: :failed, deliver_webhook: false)
 
-        raise
+        result.service_failure!(code: e.code, message: e.message)
+        nil
       end
 
       def payable_payment_status(payment_status)

spec/jobs/payment_requests/payments/gocardless_create_job_spec.rb

@@ -1,22 +1,44 @@
 # frozen_string_literal: true
 
-require 'rails_helper'
+require "rails_helper"
 
 RSpec.describe PaymentRequests::Payments::GocardlessCreateJob, type: :job do
   let(:payment_request) { create(:payment_request) }
 
   let(:gocardless_service) { instance_double(PaymentRequests::Payments::GocardlessService) }
+  let(:service_result) { BaseService::Result.new }
 
-  it 'calls the stripe create service' do
+  before do
     allow(PaymentRequests::Payments::GocardlessService).to receive(:new)
       .with(payment_request)
       .and_return(gocardless_service)
     allow(gocardless_service).to receive(:create)
-      .and_return(BaseService::Result.new)
+      .and_return(service_result)
+  end
 
+  it "calls the stripe create service" do
     described_class.perform_now(payment_request)
 
     expect(PaymentRequests::Payments::GocardlessService).to have_received(:new)
     expect(gocardless_service).to have_received(:create)
   end
+
+  it "does not send a payment requested email" do
+    expect { described_class.perform_now(payment_request) }
+      .not_to have_enqueued_mail(PaymentRequestMailer, :requested)
+  end
+
+  context "when the payment fails" do
+    let(:service_result) do
+      BaseService::Result.new.tap do |result|
+        result.payable = instance_double(PaymentRequest, payment_failed?: true)
+      end
+    end
+
+    it "sends a payment requested email" do
+      expect { described_class.perform_now(payment_request) }
+        .to have_enqueued_mail(PaymentRequestMailer, :requested)
+        .with(params: {payment_request:}, args: [])
+    end
+  end
 end

spec/services/payment_requests/payments/gocardless_service_spec.rb

@@ -121,6 +121,29 @@
       expect(invoice_2.ready_for_payment_processing).to eq(false)
     end
 
+    context "when payment request payment status is already succeeded" do
+      let(:payment_request) do
+        create(
+          :payment_request,
+          organization:,
+          customer:,
+          payment_status: "succeeded",
+          amount_cents: 799,
+          amount_currency: "EUR"
+        )
+      end
+
+      it "does not creates a payment", :aggregate_failures do
+        result = gocardless_service.create
+
+        expect(result).to be_success
+        expect(result.payable).to be_payment_succeeded
+        expect(result.payment).to be_nil
+
+        expect(gocardless_payments_service).not_to have_received(:create)
+      end
+    end
+
     context "with no payment provider" do
       let(:gocardless_payment_provider) { nil }
 
@@ -187,7 +210,7 @@
       end
 
       it "delivers an error webhook" do
-        expect { gocardless_service.create }.to raise_error(GoCardlessPro::Error)
+        gocardless_service.create
 
         expect(SendWebhookJob).to have_been_enqueued
           .with(
@@ -200,6 +223,14 @@
             }
           )
       end
+
+      it "returns a service failure" do
+        result = gocardless_service.create
+
+        expect(result).not_to be_success
+        expect(result.error.code).to eq("code")
+        expect(result.payable).to be_payment_failed
+      end
     end
 
     context "when customer has no mandate to make a payment" do
@@ -232,6 +263,14 @@
             }
           )
       end
+
+      it "marks the payment request as payment failed" do
+        result = gocardless_service.create
+
+        expect(result).not_to be_success
+        expect(result.error.code).to eq("no_mandate_error")
+        expect(payment_request.reload).to be_payment_failed
+      end
     end
   end