-
Notifications
You must be signed in to change notification settings - Fork 384
Add reconciliation job to sync stuck processing invoices with Wise #1617
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class InvoicePaymentTransferUpdate | ||
| def initialize(payment, current_state: nil, occurred_at: Time.current) | ||
| @payment = payment | ||
| @invoice = payment.invoice | ||
| @current_state = current_state | ||
| @occurred_at = occurred_at | ||
| end | ||
|
|
||
| def process | ||
| @current_state ||= fetch_transfer["status"] | ||
|
|
||
| return if current_state == payment.wise_transfer_status | ||
|
|
||
| payment.update!(wise_transfer_status: current_state) | ||
|
|
||
| if payment.in_failed_state? | ||
| unless payment.marked_failed? | ||
| payment.update!(status: Payment::FAILED) | ||
| amount_cents = fetch_transfer["sourceValue"] * -100 | ||
| payment.balance_transactions.create!(company: payment.company, amount_cents:, transaction_type: BalanceTransaction::PAYMENT_FAILED) | ||
| end | ||
| invoice.update!(status: Invoice::FAILED) | ||
| elsif payment.in_processing_state? | ||
| invoice.update!(status: Invoice::PROCESSING) | ||
| elsif current_state == Payments::Wise::OUTGOING_PAYMENT_SENT | ||
| amount = fetch_transfer["targetValue"] | ||
| estimate = Time.zone.parse(api_service.delivery_estimate(transfer_id: payment.wise_transfer_id)["estimatedDeliveryDate"]) | ||
| payment.update!(status: Payment::SUCCEEDED, wise_transfer_amount: amount, wise_transfer_estimate: estimate) | ||
| invoice.mark_as_paid!(timestamp: occurred_at, payment_id: payment.id) | ||
| end | ||
| end | ||
|
|
||
| private | ||
| attr_reader :payment, :invoice, :current_state, :occurred_at | ||
|
|
||
| def api_service | ||
| @api_service ||= Wise::PayoutApi.new(wise_credential: payment.wise_credential) | ||
| end | ||
|
|
||
| def fetch_transfer | ||
| @fetch_transfer ||= api_service.get_transfer(transfer_id: payment.wise_transfer_id) | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class ReconcileProcessingInvoicesJob | ||
| include Sidekiq::Job | ||
| sidekiq_options retry: 5 | ||
|
|
||
| def perform | ||
| Invoice.processing.find_each do |invoice| | ||
| invoice.payments.where.not(wise_transfer_id: nil).each do |payment| | ||
| InvoicePaymentTransferUpdate.new(payment).process | ||
| end | ||
| rescue => e | ||
| Rails.logger.error("Failed to reconcile invoice #{invoice.id}: #{e.message}") | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,26 +26,10 @@ def perform(params) | |
| end | ||
| return | ||
| end | ||
| invoice = payment.invoice | ||
| payment.update!(wise_transfer_status: current_state) | ||
| api_service = Wise::PayoutApi.new(wise_credential: payment.wise_credential) | ||
|
|
||
| if payment.in_failed_state? | ||
| unless payment.marked_failed? | ||
| payment.update!(status: Payment::FAILED) | ||
| if payment.is_a?(Payment) | ||
| amount_cents = api_service.get_transfer(transfer_id:)["sourceValue"] * -100 | ||
| payment.balance_transactions.create!(company: payment.company, amount_cents:, transaction_type: BalanceTransaction::PAYMENT_FAILED) | ||
| end | ||
| end | ||
| invoice.update!(status: Invoice::FAILED) | ||
| elsif payment.in_processing_state? | ||
| invoice.update!(status: Invoice::PROCESSING) | ||
| elsif current_state == Payments::Wise::OUTGOING_PAYMENT_SENT | ||
| amount = api_service.get_transfer(transfer_id:)["targetValue"] | ||
| estimate = Time.zone.parse(api_service.delivery_estimate(transfer_id:)["estimatedDeliveryDate"]) | ||
| payment.update!(status: Payment::SUCCEEDED, wise_transfer_amount: amount, wise_transfer_estimate: estimate) | ||
| invoice.mark_as_paid!(timestamp: Time.zone.parse(params.dig("data", "occurred_at")), payment_id: payment.id) | ||
| end | ||
| InvoicePaymentTransferUpdate.new( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced the inline invoice payment status handling with the new |
||
| payment, | ||
| current_state:, | ||
| occurred_at: Time.zone.parse(params.dig("data", "occurred_at")), | ||
| ).process | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,3 +72,8 @@ vesting_report_csv_email_job: | |
| cron: "0 16 1 * *" # The 1st day of every month at UTC 16:00 | ||
| class: VestingReportCsvEmailJob | ||
| description: Sends monthly vesting report to company admins | ||
|
|
||
| reconcile_processing_invoices_job: | ||
| cron: "0 */4 * * *" # Every 4 hours | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Every 4 hours is frequent enough to catch stuck invoices within a reasonable window, but conservative enough to not over-call the Wise API |
||
| class: ReconcileProcessingInvoicesJob | ||
| description: Reconciles invoices stuck in processing by checking transfer status via Wise API | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| RSpec.describe InvoicePaymentTransferUpdate do | ||
| let(:wise_credential) { create(:wise_credential) } | ||
| let(:invoice) { create(:invoice, :processing) } | ||
| let(:payment) do | ||
| create(:payment, | ||
| invoice:, | ||
| wise_credential:, | ||
| wise_transfer_id: "12345", | ||
| wise_transfer_status: Payments::Wise::PROCESSING, | ||
| status: Payment::INITIAL) | ||
| end | ||
| let(:current_time) { Time.current.change(usec: 0) } | ||
| let(:transfer_estimate) { current_time + 2.days } | ||
|
|
||
| before do | ||
| allow_any_instance_of(Wise::PayoutApi).to( | ||
| receive(:get_transfer).and_return({ "targetValue" => 50.0, "sourceValue" => 60.0 }) | ||
| ) | ||
| allow_any_instance_of(Wise::PayoutApi).to( | ||
| receive(:delivery_estimate).and_return({ "estimatedDeliveryDate" => transfer_estimate.iso8601 }) | ||
| ) | ||
| end | ||
|
|
||
| it "marks the payment as succeeded and invoice as paid for a successful transfer" do | ||
| described_class.new(payment, current_state: Payments::Wise::OUTGOING_PAYMENT_SENT, occurred_at: current_time).process | ||
|
|
||
| payment.reload | ||
| invoice.reload | ||
| expect(payment.status).to eq(Payment::SUCCEEDED) | ||
| expect(payment.wise_transfer_status).to eq(Payments::Wise::OUTGOING_PAYMENT_SENT) | ||
| expect(payment.wise_transfer_amount).to eq(50.0) | ||
| expect(payment.wise_transfer_estimate).to eq(transfer_estimate) | ||
| expect(invoice.status).to eq(Invoice::PAID) | ||
| expect(invoice.paid_at).to eq(current_time) | ||
| end | ||
|
|
||
| it "marks the payment and invoice as failed for a failed transfer" do | ||
| expect do | ||
| described_class.new(payment, current_state: Payments::Wise::CANCELLED).process | ||
| end.to change { PaymentBalanceTransaction.count }.by(1) | ||
|
|
||
| payment.reload | ||
| invoice.reload | ||
| expect(payment.status).to eq(Payment::FAILED) | ||
| expect(payment.wise_transfer_status).to eq(Payments::Wise::CANCELLED) | ||
| expect(invoice.status).to eq(Invoice::FAILED) | ||
|
|
||
| balance_transaction = payment.balance_transactions.last | ||
| expect(balance_transaction.amount_cents).to eq(-6000) | ||
| expect(balance_transaction.transaction_type).to eq(BalanceTransaction::PAYMENT_FAILED) | ||
| end | ||
|
|
||
| it "keeps the invoice in processing for an intermediary transfer state" do | ||
| described_class.new(payment, current_state: Payments::Wise::FUNDS_CONVERTED).process | ||
|
|
||
| payment.reload | ||
| invoice.reload | ||
| expect(payment.wise_transfer_status).to eq(Payments::Wise::FUNDS_CONVERTED) | ||
| expect(invoice.status).to eq(Invoice::PROCESSING) | ||
| end | ||
|
|
||
| it "does not create duplicate balance transactions when payment is already failed" do | ||
| payment.update!(status: Payment::FAILED) | ||
|
|
||
| expect do | ||
| described_class.new(payment, current_state: Payments::Wise::FUNDS_REFUNDED).process | ||
| end.not_to change { PaymentBalanceTransaction.count } | ||
|
|
||
| expect(invoice.reload.status).to eq(Invoice::FAILED) | ||
| end | ||
|
|
||
| it "defaults occurred_at to Time.current" do | ||
| freeze_time do | ||
| described_class.new(payment, current_state: Payments::Wise::OUTGOING_PAYMENT_SENT).process | ||
| expect(invoice.reload.paid_at).to eq(Time.current) | ||
| end | ||
| end | ||
|
|
||
| it "fetches current_state from Wise API when not provided" do | ||
| allow_any_instance_of(Wise::PayoutApi).to( | ||
| receive(:get_transfer).and_return({ "status" => Payments::Wise::OUTGOING_PAYMENT_SENT, "targetValue" => 50.0 }) | ||
| ) | ||
|
|
||
| described_class.new(payment).process | ||
|
|
||
| expect(payment.reload.status).to eq(Payment::SUCCEEDED) | ||
| expect(invoice.reload.status).to eq(Invoice::PAID) | ||
| end | ||
|
|
||
| it "skips processing when the transfer status has not changed" do | ||
| allow_any_instance_of(Wise::PayoutApi).to( | ||
| receive(:get_transfer).and_return({ "status" => Payments::Wise::PROCESSING }) | ||
| ) | ||
|
|
||
| described_class.new(payment).process | ||
|
|
||
| expect(payment.reload.status).to eq(Payment::INITIAL) | ||
| expect(invoice.reload.status).to eq(Invoice::PROCESSING) | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| RSpec.describe ReconcileProcessingInvoicesJob do | ||
| describe "#perform" do | ||
| let(:wise_credential) { create(:wise_credential) } | ||
| let(:api_service) { instance_double(Wise::PayoutApi) } | ||
|
|
||
| before do | ||
| allow(Wise::PayoutApi).to receive(:new).and_return(api_service) | ||
| end | ||
|
|
||
| context "when a processing invoice has a payment with a wise_transfer_id" do | ||
| let(:invoice) { create(:invoice, :processing) } | ||
| let!(:payment) do | ||
| create(:payment, | ||
| invoice:, | ||
| wise_credential:, | ||
| wise_transfer_id: "12345", | ||
| wise_transfer_status: Payments::Wise::PROCESSING, | ||
| status: Payment::INITIAL) | ||
| end | ||
|
|
||
| before do | ||
| allow(api_service).to receive(:get_transfer).with(transfer_id: "12345").and_return({ | ||
| "status" => Payments::Wise::OUTGOING_PAYMENT_SENT, | ||
| "targetValue" => 50.0, | ||
| }) | ||
| allow(api_service).to receive(:delivery_estimate).with(transfer_id: "12345").and_return({ | ||
| "estimatedDeliveryDate" => "2026-02-14T12:00:00Z", | ||
| }) | ||
| end | ||
|
|
||
| it "delegates to InvoicePaymentTransferUpdate" do | ||
| expect(InvoicePaymentTransferUpdate).to receive(:new).with(payment).and_call_original | ||
| described_class.new.perform | ||
| end | ||
| end | ||
|
|
||
| context "when there are no processing invoices" do | ||
| let!(:invoice) { create(:invoice, :paid) } | ||
|
|
||
| it "does not call the Wise API" do | ||
| expect(InvoicePaymentTransferUpdate).not_to receive(:new) | ||
| described_class.new.perform | ||
| end | ||
| end | ||
|
|
||
| context "when the payment has no wise_transfer_id" do | ||
| let(:invoice) { create(:invoice, :processing) } | ||
| let!(:payment) do | ||
| create(:payment, | ||
| invoice:, | ||
| wise_credential:, | ||
| wise_transfer_id: nil, | ||
| status: Payment::INITIAL) | ||
| end | ||
|
|
||
| it "skips the payment" do | ||
| expect(InvoicePaymentTransferUpdate).not_to receive(:new) | ||
| described_class.new.perform | ||
| end | ||
| end | ||
|
|
||
| context "when Wise API raises an error for one invoice" do | ||
| let(:invoice1) { create(:invoice, :processing) } | ||
| let(:invoice2) { create(:invoice, :processing) } | ||
| let!(:payment1) do | ||
| create(:payment, | ||
| invoice: invoice1, | ||
| wise_credential:, | ||
| wise_transfer_id: "11111", | ||
| wise_transfer_status: Payments::Wise::PROCESSING, | ||
| status: Payment::INITIAL) | ||
| end | ||
| let!(:payment2) do | ||
| create(:payment, | ||
| invoice: invoice2, | ||
| wise_credential:, | ||
| wise_transfer_id: "22222", | ||
| wise_transfer_status: Payments::Wise::PROCESSING, | ||
| status: Payment::INITIAL) | ||
| end | ||
|
|
||
| before do | ||
| allow(api_service).to receive(:get_transfer).with(transfer_id: "11111").and_raise(StandardError, "API error") | ||
| allow(api_service).to receive(:get_transfer).with(transfer_id: "22222").and_return({ | ||
| "status" => Payments::Wise::OUTGOING_PAYMENT_SENT, | ||
| "targetValue" => 50.0, | ||
| }) | ||
| allow(api_service).to receive(:delivery_estimate).with(transfer_id: "22222").and_return({ | ||
| "estimatedDeliveryDate" => "2026-02-14T12:00:00Z", | ||
| }) | ||
| end | ||
|
|
||
| it "continues processing remaining invoices" do | ||
| described_class.new.perform | ||
|
|
||
| expect(invoice1.reload.status).to eq(Invoice::PROCESSING) | ||
| expect(invoice2.reload.status).to eq(Invoice::PAID) | ||
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New service extracted from
WiseTransferUpdateJobto handle invoice payment transfer status updates to we can reuse it