From bcbbbaa580ccd5880109c61c3649d29d74fc560a Mon Sep 17 00:00:00 2001 From: Ancor Cruz Date: Sat, 9 May 2026 01:10:08 +0100 Subject: [PATCH 1/3] feat(payments): add GoCardless payment cancel service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context Payment-gated subscriptions need a way to release a pending GoCardless authorization when activation times out without the customer completing payment, so funds are not held indefinitely on the PSP side. ## Description Add PaymentProviders::Gocardless::Payments::CancelService, a leaf that calls GoCardless's payments.cancel(id) action. The sync response returns the updated Payment resource with status "cancelled", so the service mirrors the create-side pattern: write the response status onto the Payment record and map it to payable_payment_status via the provider's status mapping (cancelled is in FAILED_STATUSES, so the local lifecycle status becomes failed). Expose the payment on the result via Result = BaseResult[:payment]. GoCardlessPro::InvalidStateError — raised when the payment is in a state that cannot be cancelled (already submitted, paid out, cancelled, etc.) — is rescued and treated as a successful no-op so the caller (timeout/expiration flow) does not block on PSP-side cleanup. The Payment record is left untouched in that case; the webhook for the prior state transition is the right authority for that state. Other GoCardlessPro::Error subclasses propagate so the caller can retry. The CANCELLATION webhook flow is already wired: PaymentProviders::Gocardless::HandleEventService routes the "cancelled" payment action through the existing update_payment_status pipeline, so the asynchronous confirmation of the cancel reaches the Payment record through the same path used by every other payment state change. --- .../gocardless/payments/cancel_service.rb | 47 ++++++++ .../payments/cancel_service_spec.rb | 109 ++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 app/services/payment_providers/gocardless/payments/cancel_service.rb create mode 100644 spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb diff --git a/app/services/payment_providers/gocardless/payments/cancel_service.rb b/app/services/payment_providers/gocardless/payments/cancel_service.rb new file mode 100644 index 00000000000..494a72d296c --- /dev/null +++ b/app/services/payment_providers/gocardless/payments/cancel_service.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module PaymentProviders + module Gocardless + module Payments + class CancelService < BaseService + Result = BaseResult[:payment] + + def initialize(payment:) + @payment = payment + super + end + + def call + gocardless_result = client.payments.cancel(payment.provider_payment_id) + + payment.status = gocardless_result.status + payment.payable_payment_status = payment.payment_provider.determine_payment_status(payment.status) + payment.save! + + result.payment = payment + result + rescue GoCardlessPro::InvalidStateError => e + # Best-effort cancel: the payment is in a non-cancelable state + # (already submitted, paid out, cancelled, etc.). Log and treat as a + # successful no-op so the caller (timeout/expiration flow) does not + # block on PSP-side cleanup. The Payment record is left untouched; + # the webhook for the prior state transition will land its true + # state. + Rails.logger.info("GoCardless payment not cancelable for payment #{payment.id}: #{e.message}") + result + end + + private + + attr_reader :payment + + def client + @client ||= GoCardlessPro::Client.new( + access_token: payment.payment_provider.access_token, + environment: payment.payment_provider.environment + ) + end + end + end + end +end diff --git a/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb b/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb new file mode 100644 index 00000000000..287acafe24e --- /dev/null +++ b/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe PaymentProviders::Gocardless::Payments::CancelService do + subject(:result) { described_class.call(payment:) } + + let(:organization) { create(:organization) } + let(:customer) { create(:customer, organization:) } + let(:invoice) { create(:invoice, customer:, organization:) } + let(:payment_provider) { create(:gocardless_provider, organization:, access_token: "gc_test_token") } + let(:provider_customer) { create(:gocardless_customer, customer:, payment_provider:) } + let(:payment) do + create(:payment, payable: invoice, payment_provider:, payment_provider_customer: provider_customer, + organization:, customer:, provider_payment_id: "PM123", + payable_payment_status: :pending, status: "pending_submission") + end + + let(:gocardless_client) { instance_double(GoCardlessPro::Client) } + let(:gocardless_payments_service) { instance_double(GoCardlessPro::Services::PaymentsService) } + + before do + allow(GoCardlessPro::Client).to receive(:new).and_return(gocardless_client) + allow(gocardless_client).to receive(:payments).and_return(gocardless_payments_service) + end + + context "when the payment is cancelable" do + let(:cancel_response) do + instance_double(GoCardlessPro::Resources::Payment, id: "PM123", status: "cancelled") + end + + before do + allow(gocardless_payments_service).to receive(:cancel).and_return(cancel_response) + end + + it "calls the GoCardless cancel endpoint with the payment's provider id" do + result + + expect(gocardless_payments_service).to have_received(:cancel).with("PM123") + end + + it "constructs the client with the provider's access token and environment" do + result + + expect(GoCardlessPro::Client).to have_received(:new).with( + access_token: "gc_test_token", + environment: payment_provider.environment + ) + end + + it "returns a successful result with the payment" do + expect(result).to be_success + expect(result.payment).to eq(payment) + end + + it "writes the cancelled status from the response onto the payment" do + result + + expect(payment.reload.status).to eq("cancelled") + end + + it "maps the cancelled status to a failed payable_payment_status" do + result + + expect(payment.reload.payable_payment_status).to eq("failed") + end + end + + context "when GoCardless raises InvalidStateError" do + before do + allow(gocardless_payments_service).to receive(:cancel) + .and_raise(GoCardlessPro::InvalidStateError.new( + "message" => "Payment cancellation failed", + "code" => "invalid_state" + )) + end + + it "returns a successful result without raising" do + expect(result).to be_success + end + + it "logs the underlying error message" do + allow(Rails.logger).to receive(:info) + + result + + expect(Rails.logger).to have_received(:info) + .with(a_string_matching(/GoCardless.*not cancelable.*Payment cancellation failed/)) + end + + it "does not mutate the payment record" do + expect { result }.not_to change { payment.reload.attributes } + end + end + + context "when GoCardless raises a generic error" do + before do + allow(gocardless_payments_service).to receive(:cancel) + .and_raise(GoCardlessPro::Error.new( + "message" => "Internal server error", + "code" => "server_error" + )) + end + + it "propagates the error so the caller can retry" do + expect { result }.to raise_error(GoCardlessPro::Error) + end + end +end From a7e2ca35e669741828ecef3f2427cb6e88d7e67c Mon Sep 17 00:00:00 2001 From: Ancor Cruz Date: Thu, 14 May 2026 16:00:15 +0100 Subject: [PATCH 2/3] refactor(payments): narrow GoCardless cancel rescue + wrap Faraday errors Aligns the GoCardless cancel leaf with the same error-handling shape applied to the Stripe and Adyen leaves: - Narrow the GoCardlessPro::InvalidStateError rescue to only swallow code "cancellation_failed" (the documented "payment is in a state that cannot be cancelled" case). Other InvalidStateError codes propagate so the caller can retry or surface the failure. - Wrap Faraday::ConnectionFailed as Invoices::Payments::ConnectionError. The GoCardless gem surfaces transport errors as raw Faraday exceptions; wrapping matches the framework's retry pathway used by other PSP services. Spec covers all four paths: - cancelable: writes status to Payment, returns success - InvalidStateError with code "cancellation_failed": swallowed - InvalidStateError with a different code: propagates - Generic GoCardlessPro::Error: propagates - Faraday::ConnectionFailed: wrapped as Invoices::Payments::ConnectionError --- .../gocardless/payments/cancel_service.rb | 23 +++++++++---- .../payments/cancel_service_spec.rb | 33 ++++++++++++++++--- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/app/services/payment_providers/gocardless/payments/cancel_service.rb b/app/services/payment_providers/gocardless/payments/cancel_service.rb index 494a72d296c..2ff28abb557 100644 --- a/app/services/payment_providers/gocardless/payments/cancel_service.rb +++ b/app/services/payment_providers/gocardless/payments/cancel_service.rb @@ -21,14 +21,25 @@ def call result.payment = payment result rescue GoCardlessPro::InvalidStateError => e - # Best-effort cancel: the payment is in a non-cancelable state - # (already submitted, paid out, cancelled, etc.). Log and treat as a - # successful no-op so the caller (timeout/expiration flow) does not - # block on PSP-side cleanup. The Payment record is left untouched; - # the webhook for the prior state transition will land its true - # state. + # Best-effort cancel only for the documented "cancellation_failed" + # case — the payment is in a state that cannot be cancelled + # (already submitted, paid out, cancelled, etc.). Log and treat as + # a successful no-op so the caller (timeout/expiration flow) does + # not block on PSP-side cleanup. The Payment record is left + # untouched; the webhook for the prior state transition will land + # its true state. + # + # Other InvalidStateError codes propagate so the caller can retry + # or surface the failure. + raise unless e.code == "cancellation_failed" + Rails.logger.info("GoCardless payment not cancelable for payment #{payment.id}: #{e.message}") result + rescue Faraday::ConnectionFailed => e + # GoCardless gem surfaces transport errors as raw Faraday + # exceptions. Wrap so the caller can retry through the same path + # as other PSPs (matches the create-side error handling pattern). + raise Invoices::Payments::ConnectionError, e end private diff --git a/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb b/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb index 287acafe24e..1ec634740ff 100644 --- a/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb +++ b/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb @@ -66,12 +66,12 @@ end end - context "when GoCardless raises InvalidStateError" do + context "when GoCardless raises InvalidStateError with code cancellation_failed" do before do allow(gocardless_payments_service).to receive(:cancel) .and_raise(GoCardlessPro::InvalidStateError.new( - "message" => "Payment cancellation failed", - "code" => "invalid_state" + "message" => "This payment cannot be cancelled, its status is submitted", + "code" => "cancellation_failed" )) end @@ -85,7 +85,7 @@ result expect(Rails.logger).to have_received(:info) - .with(a_string_matching(/GoCardless.*not cancelable.*Payment cancellation failed/)) + .with(a_string_matching(/GoCardless.*not cancelable.*status is submitted/)) end it "does not mutate the payment record" do @@ -93,6 +93,20 @@ end end + context "when GoCardless raises InvalidStateError with a different code" do + before do + allow(gocardless_payments_service).to receive(:cancel) + .and_raise(GoCardlessPro::InvalidStateError.new( + "message" => "Mandate is inactive", + "code" => "mandate_is_inactive" + )) + end + + it "propagates the error so the caller can retry or surface the failure" do + expect { result }.to raise_error(GoCardlessPro::InvalidStateError) + end + end + context "when GoCardless raises a generic error" do before do allow(gocardless_payments_service).to receive(:cancel) @@ -106,4 +120,15 @@ expect { result }.to raise_error(GoCardlessPro::Error) end end + + context "when a Faraday connection failure occurs" do + before do + allow(gocardless_payments_service).to receive(:cancel) + .and_raise(Faraday::ConnectionFailed.new("connection refused")) + end + + it "wraps the error as Invoices::Payments::ConnectionError so the caller can retry" do + expect { result }.to raise_error(Invoices::Payments::ConnectionError) + end + end end From 8698bffc036e9cf6c4eced0e07bc5ffaf9a76e10 Mon Sep 17 00:00:00 2001 From: Ancor Cruz Date: Tue, 19 May 2026 13:07:00 +0100 Subject: [PATCH 3/3] fix(payments): expose payment on result in GoCardless cancel rescue path The InvalidStateError rescue branch returned a successful result but did not set result.payment, so callers introspecting the rescue path got nil. The happy path sets it correctly. This inconsistency was caught in review (PR #5484). Tighten the spec assertion in the rescue context from "returns a successful result without raising" to "returns a successful result with the payment" so the contract is locked in and a future regression would fail the test. (Stripe leaf has the same gap and will be addressed in a separate small PR.) --- .../payment_providers/gocardless/payments/cancel_service.rb | 1 + .../gocardless/payments/cancel_service_spec.rb | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/services/payment_providers/gocardless/payments/cancel_service.rb b/app/services/payment_providers/gocardless/payments/cancel_service.rb index 2ff28abb557..06249b27f38 100644 --- a/app/services/payment_providers/gocardless/payments/cancel_service.rb +++ b/app/services/payment_providers/gocardless/payments/cancel_service.rb @@ -34,6 +34,7 @@ def call raise unless e.code == "cancellation_failed" Rails.logger.info("GoCardless payment not cancelable for payment #{payment.id}: #{e.message}") + result.payment = payment result rescue Faraday::ConnectionFailed => e # GoCardless gem surfaces transport errors as raw Faraday diff --git a/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb b/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb index 1ec634740ff..043fb82988f 100644 --- a/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb +++ b/spec/services/payment_providers/gocardless/payments/cancel_service_spec.rb @@ -75,8 +75,9 @@ )) end - it "returns a successful result without raising" do + it "returns a successful result with the payment" do expect(result).to be_success + expect(result.payment).to eq(payment) end it "logs the underlying error message" do