Skip to content

feature/INT-1675 - BACS Direct Debit notifications + refactor - #199

Merged
david-ruiz-cko merged 4 commits into
masterfrom
feature/INT-1675
Sep 4, 2026
Merged

feature/INT-1675 - BACS Direct Debit notifications + refactor#199
david-ruiz-cko merged 4 commits into
masterfrom
feature/INT-1675

Conversation

@david-ruiz-cko

@david-ruiz-cko david-ruiz-cko commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Breaking changes (check at the end image)

This pull request introduces support for Bacs Direct Debit and ACH payment instruments, as well as improvements and clarifications to the SDK's type documentation. The main changes include adding Bacs and ACH instrument models, enabling Bacs notifications, and updating type definitions to support new use cases and improve clarity.

Bacs Direct Debit support:

  • Added Bacs Direct Debit client (BacsClient) with support for sending pre-notification requests (send_notification) and corresponding request/response types. (lib/checkout_sdk/apm/apm.rb, lib/checkout_sdk/apm/bacs/bacs_client.rb, lib/checkout_sdk/apm/bacs/bacs_notification_request.rb, lib/checkout_sdk/apm/bacs/bacs_notification_type.rb) [1] [2] [3] [4]
  • Registered the Bacs client in CheckoutApi, making it available as api.bacs. (lib/checkout_sdk/checkout_api.rb) [1] [2] [3]
  • Added models for creating Bacs instruments, including account details, billing address, account holder, and payment type. (lib/checkout_sdk/instruments/create/create_bacs_instrument_data.rb, lib/checkout_sdk/instruments/create/create_bacs_billing_address.rb, lib/checkout_sdk/instruments/create/create_bacs_account_holder.rb, lib/checkout_sdk/instruments/create/create_bacs_instrument_account.rb, lib/checkout_sdk/instruments/bacs_payment_type.rb) [1] [2] [3] [4] [5]
  • Added BACS as a supported payment source and instrument type. (lib/checkout_sdk/common/payment_source_type.rb, lib/checkout_sdk/common/instrument_type.rb) [1] [2]

ACH Direct Debit support:

  • Added models for creating ACH instruments, including account holder details, instrument data, and account type. (lib/checkout_sdk/instruments/create/create_ach_account_holder.rb, lib/checkout_sdk/instruments/create/create_ach_instrument_data.rb, lib/checkout_sdk/instruments/ach_account_type.rb) [1] [2] [3]
  • Added ACH as a supported instrument and payment source type. (lib/checkout_sdk/common/instrument_type.rb, lib/checkout_sdk/common/payment_source_type.rb) [1] [2]

SEPA Direct Debit improvements:

  • Added models for creating SEPA instruments, including account holder and billing address. (lib/checkout_sdk/instruments/create/create_sepa_account_holder.rb, lib/checkout_sdk/instruments/create/create_sepa_billing_address.rb) [1] [2]

Type and documentation enhancements:

  • Improved documentation for AccountHolderType, CustomerRequest, and InstrumentType, clarifying usage and adding new values such as GOVERNMENT and BACS. (lib/checkout_sdk/common/account_holder_type.rb, lib/checkout_sdk/common/customer_request.rb, lib/checkout_sdk/common/instrument_type.rb) [1] [2] [3]

These changes add significant new functionality for direct debit payments and improve the SDK's clarity and extensibility.

Breaking changes imageimageimage

Two, plus a set of behaviour changes that make previously-invalid requests valid. Everything else in this PR is additive.

1. CheckoutSdk::Instruments::InstrumentData has been removed

It was a single class serving the SEPA instrument's instrument_data, and it carried two defects:

  • Its payment_type YARD pointed at CheckoutSdk::Payments::PaymentType — the payment-level enum (Regular, Recurring, Moto, Installment, Unscheduled). SEPA instruments declare payment_type as lowercase recurring / regular, so following the documentation produced a request the API rejects. The repo's own integration spec did exactly that.
  • Its date_of_signature was documented as DateTime. The field is format: date and the API expects yyyy-MM-dd, so a DateTime serialized to a full timestamp.

It is replaced by per-scheme, spec-literal classes. The store and update shapes are genuinely different in the specification — different required sets and different maxLength values on the same fields — so they are separate types rather than one shared one.

Migration

# before
data = CheckoutSdk::Instruments::InstrumentData.new
data.account_number = 'FR7630006000011234567890189'
data.country = CheckoutSdk::Common::Country::FR
data.currency = CheckoutSdk::Common::Currency::EUR
data.payment_type = CheckoutSdk::Payments::PaymentType::RECURRING   # 'Recurring' - wrong for SEPA
data.date_of_signature = DateTime.now                               # wrong format

request = CheckoutSdk::Instruments::InstrumentSepa.new
request.instrument_data = data
request.account_holder = account_holder                             # Common::AccountHolder

# after
data = CheckoutSdk::Instruments::CreateSepaInstrumentData.new
data.account_number = 'FR7630006000011234567890189'
data.country = CheckoutSdk::Common::Country::FR
data.currency = CheckoutSdk::Common::Currency::EUR
data.payment_type = CheckoutSdk::Instruments::SepaPaymentType::RECURRING   # 'recurring'
data.type = CheckoutSdk::Instruments::SepaMandateType::CORE                # optional
data.date_of_signature = '2026-07-15'                                      # yyyy-MM-dd String

billing_address = CheckoutSdk::Instruments::CreateSepaBillingAddress.new
billing_address.address_line1 = 'Cloverfield St.'
billing_address.address_line2 = '23A'
billing_address.city = 'London'
billing_address.zip = 'SW1A 1AA'
billing_address.country = CheckoutSdk::Common::Country::GB

account_holder = CheckoutSdk::Instruments::CreateSepaAccountHolder.new
account_holder.first_name = 'John'
account_holder.last_name = 'Smith'
account_holder.billing_address = billing_address

request = CheckoutSdk::Instruments::InstrumentSepa.new
request.instrument_data = data
request.account_holder = account_holder

InstrumentSepa itself is unchanged apart from gaining the customer attribute the specification declares. Because the SDK is duck-typed, a Hash also works if you prefer not to adopt the new classes.

2. CheckoutSdk::Instruments::PaymentNetwork values changed casing

Four of six values were wrong on the wire. The constant names are unchanged, so nothing fails to load — but the string sent to the API is different:

constant before after
PaymentNetwork::FPS 'Fps' 'fps'
PaymentNetwork::ACH 'Ach' 'ach'
PaymentNetwork::FEDWIRE 'Fedwire' 'fedwire'
PaymentNetwork::SWIFT 'Swift' 'swift'

LOCAL and SEPA were already correct. These are query-parameter values on GET /validation/bank-accounts/{country}/{currency}; the specification declares all six lowercase, so the four capitalized ones were silently ignored as a filter rather than rejected. If you were passing them and relying on the unfiltered result, the response will now be narrower — which is the documented behaviour. If you hardcoded the lowercase strings yourself, nothing changes for you.

Behaviour changes that fix previously-invalid requests

Neither is a compile or load break in Ruby, but both change what a correct integration should send.

  • Payments::AchSource#account_type now documents Payments::AchSourceAccountType (savings / checking / cash). It previously pointed at Common::AccountType, which is savings / current / cash. PaymentRequestAchSource is the only schema in the specification declaring checking, and it does not accept current — so Common::AccountType::CURRENT never produced a valid ACH payment source, and checking could not be named at all. Common::AccountType is unchanged and remains correct for the seven bank-account positions that do declare current.
  • date_of_signature on both SEPA instrument-data classes is a yyyy-MM-dd String, not a DateTime. See above.

What's new (additive)

cko.bacs.send_notification(request)POST /apms/bacs/notifications, secret key. Sends a Bacs Direct Debit pre-notification. Registered on the current-API CheckoutApi only; the previous platform does not expose it.

request = CheckoutSdk::Apm::BacsNotificationRequest.new
request.source_id = 'src_wmlfc3zyhqzehihu7giusaaawu'
request.notification_type = CheckoutSdk::Apm::BacsNotificationType::ADVANCE_NOTICE
request.collection_date = '2026-07-15'
request.amount = 4999
request.currency = CheckoutSdk::Common::Currency::GBP
request.billing_descriptor = 'CHECKOUT'
request.customer_email = 'customer@example.com'
request.support_email = 'support@test.com'
# optional: reference, support_phone

response = cko.bacs.send_notification(request)
response.event_id

Bacs, SEPA and ACH instrumentsInstrumentBacs, InstrumentAch, UpdateInstrumentBacs, UpdateInstrumentSepa, UpdateInstrumentAch, plus 15 value classes and 5 enums (SepaPaymentType, BacsPaymentType, SepaMandateType, AchAccountType, InstrumentAccountHolderType).

Payment sourcesBacsSource, plus the 12 APM variants that had no dedicated class: AlipayCnSource, AlipayHkSource, DanaSource, GcashSource, KakaopaySource, MobilePaySource, PayNowSource, SwishSource (with SwishAccountHolder and SwishBillingDescriptor), TngSource, TruemoneySource, TwintSource, VippsSource. Ruby now covers all 39 request-source variants the specification declares.

Seven of those wire types were already reachable through AlipayPlusSource factory methods (alipay_plus_cn_source and friends). Those factories are retained and still work; a test asserts they produce byte-identical JSON to the new dedicated classes. Prefer the dedicated class going forward.

SepaSource#mandate_type — the Core / B2B mandate type the SEPA payment source declares and the SDK could not send.

Enum additionsCommon::InstrumentType: ACH, BACS. Common::PaymentSourceType: BACS, MOBILEPAY, PAYNOW, SWISH, TWINT, VIPPS. Common::AccountHolderType: GOVERNMENT.

Two casing traps documented in the code

Both are real specification behaviour, not SDK quirks, and both now carry regression tests so they cannot be "tidied up" later:

  • Bacs payment_type is capitalized (Recurring / Regular); SEPA payment_type is lowercase (recurring / regular). Two separate enums; the values are not interchangeable.
  • account_holder.type on the SEPA payment source: the specification declares Individual / Corporate capitalized at that one position, against 23 lowercase account-holder-type positions elsewhere — including the sibling ACH source. Send lowercase. Every Checkout.com SDK sends lowercase, and this is pending confirmation from the API owners as a likely specification defect. Documented on the attribute.

@david-ruiz-cko
david-ruiz-cko requested a review from a team September 2, 2026 13:13
@agent-wall-e

agent-wall-e Bot commented Sep 2, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • file_removed:lib/checkout_sdk/instruments/create/instrument_data.rb

Operational gates

  • ✅ jira_ticket (INT-1675)
  • ✅ independent_review

Files analysed: 50


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Sep 2, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
file_removedlib/checkout_sdk/instruments/create/instrument_data.rb classifying §2.1 M1 A non-doc/test/lockfile was deleted — destructive change.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@david-ruiz-cko david-ruiz-cko changed the title eature/INT-1675 - BACS Direct Debit notifications + refactor feature/INT-1675 - BACS Direct Debit notifications + refactor Sep 2, 2026
@agent-wall-e

agent-wall-e Bot commented Sep 2, 2026

Copy link
Copy Markdown

🔵 Advisory review: Sound, but needs your judgement

This PR needs a human approval. The code itself reads as correct; whether it should land depends on context I don't have.

Large additive SDK expansion adding Bacs/ACH/SEPA instrument and payment source models, a BacsClient, and documentation improvements; the code looks structurally correct but the diff is explicitly truncated (18 files omitted) and contains one known breaking change (PaymentNetwork casing) that callers may depend on.

For you to decide

  • The diff is truncated — 18 files are omitted, so completeness cannot be verified; the reviewer must check those files before approving.
  • PaymentNetwork constants FPS/ACH/FEDWIRE/SWIFT are changed from title-case ('Fps', 'Ach', etc.) to lowercase; any caller passing these values to the API today will silently send a different string after upgrade, and the PR description does not flag this as a breaking change.
  • SepaSourceAccountHolder documents that type should be sent lowercase despite the spec declaring it capitalized, with a note 'pending confirmation from API owners' — this is an unresolved API ambiguity that should be confirmed before shipping.
  • BacsNotificationRequest is a plain data struct with no validation; callers who omit required fields (source_id, notification_type, collection_date, amount, currency, billing_descriptor, customer_email, support_email) will get a runtime error from the API rather than a local error, which is consistent with the rest of the SDK but worth the reviewer confirming is intentional.
  • The customer attribute added to InstrumentSepa is a behaviour change for existing SEPA users (previously impossible to set), which the PR description acknowledges as a 'previously-invalid request becoming valid' — reviewer should confirm this is safe for existing serialization paths.
  • No tests are visible in the diff; the reviewer should confirm test coverage exists for BacsClient#send_notification, the new instrument classes, and the PaymentNetwork casing change.

⚠️ The diff was too large to read in full, so this review covers only part of the change.


This is not an approval. wall-e cannot auto-approve this PR — it is an opinion to help whoever does. Advisory review · us.anthropic.claude-sonnet-4-6 · wall-e 2026.06.19-02

@agent-wall-e

agent-wall-e Bot commented Sep 2, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • file_removed:lib/checkout_sdk/instruments/create/instrument_data.rb

Operational gates

  • ✅ jira_ticket (INT-1675)
  • ✅ independent_review

Files analysed: 69


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Sep 2, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
file_removedlib/checkout_sdk/instruments/create/instrument_data.rb classifying §2.1 M1 A non-doc/test/lockfile was deleted — destructive change.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@agent-wall-e

agent-wall-e Bot commented Sep 2, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • file_removed:lib/checkout_sdk/instruments/create/instrument_data.rb

Operational gates

  • ✅ jira_ticket (INT-1675)
  • ✅ independent_review

Files analysed: 70


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Sep 2, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
file_removedlib/checkout_sdk/instruments/create/instrument_data.rb classifying §2.1 M1 A non-doc/test/lockfile was deleted — destructive change.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@agent-wall-e

agent-wall-e Bot commented Sep 3, 2026

Copy link
Copy Markdown

🔴 Risk Classification: MAJOR

Approval route: AI Review + Human Approval Required
Rollback controls: Change-freeze window + documented rollback plan

Classification reasons

  • file_removed:lib/checkout_sdk/instruments/create/instrument_data.rb

Operational gates

  • ✅ jira_ticket (INT-1675)
  • ✅ independent_review

Files analysed: 74


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Sep 3, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
file_removedlib/checkout_sdk/instruments/create/instrument_data.rb classifying §2.1 M1 A non-doc/test/lockfile was deleted — destructive change.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

@david-ruiz-cko
david-ruiz-cko merged commit da1df21 into master Sep 4, 2026
5 checks passed
@david-ruiz-cko
david-ruiz-cko deleted the feature/INT-1675 branch September 4, 2026 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants