Skip to content

Commit

Permalink
fix(gocardless): Treat Gocardless webhooks events in separate jobs (#…
Browse files Browse the repository at this point in the history
…2695)

This pull request focuses on improving the GoCardless webhook events
management and refactoring the integration by introducing new services
for handling webhooks and events using the standard call/Result pattern.

The main changes include replacing direct calls to `GocardlessService`
with more specific services and updating the logic to treat each webhook
events in isolation to avoid failling a batch if a single event is
causing an error.

### Refactoring and Service Extraction:

* **Refactored Webhook Handling:**
- Replaced `GocardlessService.handle_incoming_webhook` with
`Gocardless::HandleIncomingWebhookService.call` in `WebhooksController`.
(`app/controllers/webhooks_controller.rb`)
- Extracted webhook handling logic into a new service
`HandleIncomingWebhookService`.
(`app/services/payment_providers/gocardless/handle_incoming_webhook_service.rb`)

* **Refactored Event Handling:**
- Replaced `GocardlessService.handle_event` with
`Gocardless::HandleEventService.call` in `HandleEventJob`.
(`app/jobs/payment_providers/gocardless/handle_event_job.rb`)
- Extracted event handling logic into a new service
`HandleEventService`.
(`app/services/payment_providers/gocardless/handle_event_service.rb`)
- Keep acception the legacy `events_json` input argument in
`HandleEventJob` to avoid issues with enqueued/dead jobs for now
  • Loading branch information
vincent-pochet authored Oct 17, 2024
1 parent 52dff29 commit 787c6ed
Show file tree
Hide file tree
Showing 10 changed files with 299 additions and 253 deletions.
2 changes: 1 addition & 1 deletion app/controllers/webhooks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def stripe
end

def gocardless
result = PaymentProviders::GocardlessService.new.handle_incoming_webhook(
result = PaymentProviders::Gocardless::HandleIncomingWebhookService.call(
organization_id: params[:organization_id],
code: params[:code].presence,
body: request.body.read,
Expand Down
14 changes: 11 additions & 3 deletions app/jobs/payment_providers/gocardless/handle_event_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,17 @@ module Gocardless
class HandleEventJob < ApplicationJob
queue_as 'providers'

def perform(events_json:)
result = PaymentProviders::GocardlessService.new.handle_event(events_json:)
result.raise_if_error!
def perform(organization: nil, events_json: nil, event_json: nil)
# NOTE: temporary keeps both events_json and event_json to avoid errors during the deployment
if events_json.present?
JSON.parse(events_json)['events'].each do |event|
PaymentProviders::Gocardless::HandleEventJob.perform_later(event_json: event.to_json)
end

return
end

PaymentProviders::Gocardless::HandleEventService.call(event_json:).raise_if_error!
end
end
end
Expand Down
61 changes: 61 additions & 0 deletions app/services/payment_providers/gocardless/handle_event_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# frozen_string_literal: true

module PaymentProviders
module Gocardless
class HandleEventService < BaseService
PAYMENT_ACTIONS = %w[paid_out failed cancelled customer_approval_denied charged_back].freeze
REFUND_ACTIONS = %w[created funds_returned paid refund_settled failed].freeze

PAYMENT_SERVICE_CLASS_MAP = {
"Invoice" => Invoices::Payments::GocardlessService,
"PaymentRequest" => PaymentRequests::Payments::GocardlessService
}.freeze

def initialize(event_json:)
@event_json = event_json

super
end

def call
case event.resource_type
when 'payments'
if PAYMENT_ACTIONS.include?(event.action)
payment_service_klass(event)
.new.update_payment_status(
provider_payment_id: event.links.payment,
status: event.action
).raise_if_error!
end
when 'refunds'
if REFUND_ACTIONS.include?(event.action)
CreditNotes::Refunds::GocardlessService
.new.update_status(
provider_refund_id: event.links.refund,
status: event.action,
metadata: event.metadata
).raise_if_error!
end
end

result
end

private

attr_reader :organization, :event_json

def event
@event ||= GoCardlessPro::Resources::Event.new(JSON.parse(event_json))
end

def payment_service_klass(event)
payable_type = event.metadata["lago_payable_type"] || "Invoice"

PAYMENT_SERVICE_CLASS_MAP.fetch(payable_type) do
raise NameError, "Invalid lago_payable_type: #{payable_type}"
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# frozen_string_literal: true

module PaymentProviders
module Gocardless
class HandleIncomingWebhookService < BaseService
def initialize(organization_id:, body:, signature:, code: nil)
@organization_id = organization_id
@body = body
@signature = signature
@code = code

super
end

def call
payment_provider_result = PaymentProviders::FindService.call(
organization_id:,
code:,
payment_provider_type: 'gocardless'
)
return payment_provider_result unless payment_provider_result.success?

result.events = GoCardlessPro::Webhook.parse(
request_body: body,
signature_header: signature,
webhook_endpoint_secret: payment_provider_result.payment_provider&.webhook_secret
)

result.events.each do |event|
PaymentProviders::Gocardless::HandleEventJob.perform_later(
organization: payment_provider_result.payment_provider.organization,
event_json: event.to_json
)
end

result
rescue JSON::ParserError
result.service_failure!(code: 'webhook_error', message: 'Invalid payload')
rescue GoCardlessPro::Webhook::InvalidSignatureError
result.service_failure!(code: 'webhook_error', message: 'Invalid signature')
end

private

attr_reader :organization_id, :body, :signature, :code
end
end
end
79 changes: 0 additions & 79 deletions app/services/payment_providers/gocardless_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,6 @@
module PaymentProviders
class GocardlessService < BaseService
REDIRECT_URI = "#{ENV["LAGO_OAUTH_PROXY_URL"]}/gocardless/callback".freeze
PAYMENT_ACTIONS = %w[paid_out failed cancelled customer_approval_denied charged_back].freeze
REFUND_ACTIONS = %w[created funds_returned paid refund_settled failed].freeze

PAYMENT_SERVICE_CLASS_MAP = {
"Invoice" => Invoices::Payments::GocardlessService,
"PaymentRequest" => PaymentRequests::Payments::GocardlessService
}.freeze

def create_or_update(**args)
access_token = if args[:access_code].present?
Expand Down Expand Up @@ -53,70 +46,6 @@ def create_or_update(**args)
result.service_failure!(code: 'internal_error', message: e.description)
end

def handle_incoming_webhook(organization_id:, body:, signature:, code: nil)
payment_provider_result = PaymentProviders::FindService.call(
organization_id:,
code:,
payment_provider_type: 'gocardless'
)

return payment_provider_result unless payment_provider_result.success?

events = GoCardlessPro::Webhook.parse(
request_body: body,
signature_header: signature,
webhook_endpoint_secret: payment_provider_result.payment_provider&.webhook_secret
)

PaymentProviders::Gocardless::HandleEventJob.perform_later(events_json: body)

result.events = events
result
rescue JSON::ParserError
result.service_failure!(code: 'webhook_error', message: 'Invalid payload')
rescue GoCardlessPro::Webhook::InvalidSignatureError
result.service_failure!(code: 'webhook_error', message: 'Invalid signature')
end

def handle_event(events_json:)
handled_events = []
events = JSON.parse(events_json)['events']
parsed_events = events.map { |event| GoCardlessPro::Resources::Event.new(event) }
parsed_events.each do |event|
case event.resource_type
when 'payments'
if PAYMENT_ACTIONS.include?(event.action)
update_payment_status_result = payment_service_klass(event)
.new.update_payment_status(
provider_payment_id: event.links.payment,
status: event.action
)

return update_payment_status_result unless update_payment_status_result.success?

handled_events << event
end
when 'refunds'
if REFUND_ACTIONS.include?(event.action)
status_result = CreditNotes::Refunds::GocardlessService
.new.update_status(
provider_refund_id: event.links.refund,
status: event.action,
metadata: event.metadata
)

return status_result unless status_result.success?

handled_events << event
end

end
end

result.handled_events = handled_events
result
end

private

def oauth
Expand All @@ -129,13 +58,5 @@ def oauth
auth_scheme: :request_body
)
end

def payment_service_klass(event)
payable_type = event.metadata["lago_payable_type"] || "Invoice"

PAYMENT_SERVICE_CLASS_MAP.fetch(payable_type) do
raise NameError, "Invalid lago_payable_type: #{payable_type}"
end
end
end
end
34 changes: 23 additions & 11 deletions spec/jobs/payment_providers/gocardless/handle_event_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,33 @@
let(:result) { BaseService::Result.new }
let(:organization) { create(:organization) }

let(:gocardless_events) do
[]
let(:event_json) do
path = Rails.root.join('spec/fixtures/gocardless/events.json')
JSON.parse(File.read(path))['events'].first.to_json
end

before do
allow(PaymentProviders::GocardlessService).to receive(:new)
.and_return(gocardless_service)
allow(gocardless_service).to receive(:handle_event)
.and_return(result)
let(:service_result) { BaseService::Result.new }

it 'delegate to the event service' do
allow(PaymentProviders::Gocardless::HandleEventService).to receive(:call)
.with(event_json:)
.and_return(service_result)

handle_event_job.perform_now(organization:, event_json:)

expect(PaymentProviders::Gocardless::HandleEventService).to have_received(:call)
end

it 'calls the handle event service' do
handle_event_job.perform_now(events_json: gocardless_events)
context 'with legacy multiple events' do
let(:events_json) do
path = Rails.root.join('spec/fixtures/gocardless/events.json')
File.read(path)
end

it 'enqueues a job for each event' do
handle_event_job.perform_now(events_json:)

expect(PaymentProviders::GocardlessService).to have_received(:new)
expect(gocardless_service).to have_received(:handle_event)
expect(described_class).to have_been_enqueued.exactly(JSON.parse(events_json)['events'].count).times
end
end
end
10 changes: 3 additions & 7 deletions spec/requests/webhooks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,7 @@
end

before do
allow(PaymentProviders::GocardlessService).to receive(:new)
.and_return(gocardless_service)
allow(gocardless_service).to receive(:handle_incoming_webhook)
allow(PaymentProviders::Gocardless::HandleIncomingWebhookService).to receive(:call)
.with(
organization_id: organization.id,
code: nil,
Expand All @@ -128,8 +126,7 @@

expect(response).to have_http_status(:success)

expect(PaymentProviders::GocardlessService).to have_received(:new)
expect(gocardless_service).to have_received(:handle_incoming_webhook)
expect(PaymentProviders::Gocardless::HandleIncomingWebhookService).to have_received(:call)
end

context 'when failing to handle gocardless event' do
Expand All @@ -149,8 +146,7 @@

expect(response).to have_http_status(:bad_request)

expect(PaymentProviders::GocardlessService).to have_received(:new)
expect(gocardless_service).to have_received(:handle_incoming_webhook)
expect(PaymentProviders::Gocardless::HandleIncomingWebhookService).to have_received(:call)
end
end
end
Expand Down
Loading

0 comments on commit 787c6ed

Please sign in to comment.