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

[david-ruiz-cko]

Breaking changes (check at the end )

This pull request adds comprehensive support for Bacs Direct Debit to the SDK, including new models, client methods, and tests. It introduces the ability to send Bacs pre-notification requests, updates enums and instrument data structures to support Bacs, and adds serialization and integration tests to ensure correctness. The changes also clarify and expand SEPA and ACH instrument handling for consistency.

Bacs Direct Debit Support

• Added BacsNotificationRequest and BacsNotificationType models for Bacs pre-notification requests, with full property validation and documentation.
• Introduced BacsClient with a send_notification method to POST Bacs notifications, and exposed it via the main API client. [1] [2] [3]
• Added RequestBacsSource for payment requests using Bacs instruments.

Enums and Instrument Data Model Updates

• Extended PaymentSourceType and InstrumentType enums to include Bacs. Added specific enums for Bacs and SEPA payment types, and clarified account holder types. [1] [2] [3]
• Refactored instrument data structures to add dedicated classes for Bacs, SEPA, and ACH, including billing address and account holder models, and created request/response classes for creating and updating these instruments. [1] [2] [3] [4] [5]

Testing and Serialization

• Added unit, integration, and serialization tests for Bacs notification requests, covering all properties, optional fields, and enum values. [1] [2] [3]
• Ensured the Bacs client is only exposed on the current platform API, not the legacy one.

SEPA and ACH Improvements

• Clarified the difference between legacy and current SEPA source classes, and improved documentation. [1] [2]
• Refined the handling and updating of SEPA and ACH instrument data and requests for consistency with Bacs. [1] [2] [3]

Minor Fixes and Cleanups

• Corrected and clarified enum values for payment networks and account holder types.

These changes collectively enable robust Bacs Direct Debit support, align SEPA/ACH handling, and ensure the SDK is well-tested for these new capabilities.

Breaking changes

The instruments models were reshaped so that each scheme (SEPA, Bacs, ACH) and each operation has its
own type. Previously a single InstrumentData served both SEPA and ACH, with comments marking
which attributes belonged to which scheme, and the shared AccountHolder was reused on schemas that
declare a fraction of its 15 attributes.

Removed classes

Removed	Replacement
checkout_sdk.instruments.instruments.InstrumentData	SepaInstrumentData (SEPA) / AchInstrumentData (ACH)

The removed class carried nine attributes across two schemes: account_number, country,
currency, payment_type, mandate_id, date_of_signature, type (SEPA) and account_type,
bank_code (ACH). All are present on the replacements.

Removed attributes

Class	Attribute	Why
CreateSepaInstrumentRequest	token	Not declared by StoreSepaInstrumentRequest.
CreateBankAccountInstrumentRequest	bank_details	The schema declares this object as bank. The class already had a correct bank attribute, so bank_details serialized under a key the API does not declare. Use bank.
UpdateBankAccountInstrumentRequest	bank_details	Same, but this class had only bank_details, so the bank details could not be sent at all. Renamed to bank.

Retyped attributes

Class	Attribute	Before	After
CreateSepaInstrumentRequest	instrument_data	InstrumentData	SepaInstrumentData
CreateSepaInstrumentRequest	account_holder	AccountHolder (15 attrs)	SepaAccountHolder (5 attrs)
CreateAchInstrumentRequest	instrument_data	InstrumentData	AchInstrumentData
CreateAchInstrumentRequest	account_holder	AccountHolder (15 attrs)	AchAccountHolder (4 attrs, no billing address)
SepaInstrumentData	date_of_signature	datetime (on the old class)	str

Changed enum values

PaymentNetwork — four wire values corrected to the lowercase forms the
payment-network query parameter declares. The member names are unchanged, so this is source
compatible:

Member	Before	After
PaymentNetwork.FPS	'Fps'	'fps'
PaymentNetwork.ACH	'Ach'	'ach'
PaymentNetwork.FEDWIRE	'Fedwire'	'fedwire'
PaymentNetwork.SWIFT	'Swift'	'swift'

Mitigating fact: the four old values were rejected by the API, so no working integration can have
depended on them. Filtering bank-account fields by FPS, ACH, Fedwire or SWIFT was silently broken
before this change.

Why this is worth a major

Three of these were live defects, not tidying:

1. payment_type sent a value the API rejects. InstrumentData.payment_type was typed
   checkout_sdk.payments.payments.PaymentType, whose values are capitalised ('Recurring',
   'Regular'). The specification declares the SEPA field lowercase. Anyone following the annotation
   was sending a rejected value. Split into SepaPaymentType (lowercase) and BacsPaymentType
   (capitalised, which is what Bacs Direct Debit genuinely uses), with a regression test pinning both
   so they cannot be unified again.
2. PaymentNetwork — as above.
3. bank_details never reached the API, and on the update request there was no way to send the
   bank details at all.

Migration

# Before
from checkout_sdk.instruments.instruments import CreateSepaInstrumentRequest, InstrumentData
from checkout_sdk.payments.payments import PaymentType
from checkout_sdk.common.common import AccountHolder

data = InstrumentData()
data.account_number = 'FR7630006000011234567890189'
data.country = Country.FR
data.currency = Currency.EUR
data.payment_type = PaymentType.RECURRING        # sent 'Recurring' - rejected

holder = AccountHolder()
holder.first_name = 'John'
holder.last_name = 'Smith'
holder.phone = phone                             # not in the SEPA schema
holder.billing_address = address

request = CreateSepaInstrumentRequest()
request.instrument_data = data
request.account_holder = holder

# After
from checkout_sdk.common.enums import SepaMandateType, SepaPaymentType
from checkout_sdk.instruments.instruments import (
    CreateSepaInstrumentRequest, SepaAccountHolder, SepaBillingAddress, SepaInstrumentData,
)

data = SepaInstrumentData()
data.type = SepaMandateType.CORE                 # new: was on the old class but undocumented
data.account_number = 'FR7630006000011234567890189'
data.country = Country.FR
data.currency = Currency.EUR
data.payment_type = SepaPaymentType.RECURRING    # sends 'recurring'
data.date_of_signature = '2020-01-01'            # str, not datetime - see below

billing_address = SepaBillingAddress()
billing_address.address_line1 = 'Evergreen Terrace'
billing_address.address_line2 = '742'
billing_address.city = 'Paris'
billing_address.zip = '75000'
billing_address.country = Country.FR

holder = SepaAccountHolder()
holder.first_name = 'John'
holder.last_name = 'Smith'
holder.billing_address = billing_address

request = CreateSepaInstrumentRequest()
request.instrument_data = data
request.account_holder = holder
Two things to watch when upgrading

[agent-wall-e]

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

• exceeds_bounded_scope:343>200

Operational gates

• ✅ jira_ticket (INT-1675)
• ✅ independent_review

Files analysed: 12

---

_{wall-e 2026.06.19-02 · policy 376219bc71e6…}

[agent-wall-e]

🔬 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
exceeds_bounded_scope — 343>200	classifying	§2.1 M8	More than 200 non-test, non-doc, non-lockfile lines changed.

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]

🟠 Advisory review: Concerns worth a look

This PR needs a human approval. Before you give it, these are the things I'd want resolved.

Adds Bacs Direct Debit notification support with new models, client, enums, and tests; also refactors SEPA/ACH instrument data structures as breaking changes. The code is generally well-structured, but there are several concrete issues to resolve.

Concerns

• In checkout_sdk/apm/bacs.py, BacsNotificationRequest declares all properties as class-level annotations with no __init__, meaning every instance shares nothing and properties are just type hints — this is the same pattern used elsewhere in the SDK, but reference and support_phone are documented as optional yet they appear in the annotation list like all required fields; the serialization test deletes them with del request.reference, which only works if the caller explicitly set them first, so a caller who never sets them would silently omit required fields with no validation error.
• The BacsNotificationRequest docstring says reference and support_phone are the only optional properties, but customer_email, billing_descriptor, and support_email are not validated or enforced as required either — the class provides no runtime enforcement of required vs optional, so the API will reject under-populated requests with no SDK-level error.
• In bacs_integration_test.py, the integration test omits reference (optional, fine) but also omits support_phone, yet the full serialization test's comment says support_phone is optional — this is consistent, but the integration test also never asserts anything beyond event_id on the response, so it would not catch a malformed response body.
• The checkout_apm_api.py adds self.bacs to CheckoutApmApi, which is the APM-specific API; the test in checkout_api_test.py verifies api.bacs is not None on the default API and not hasattr(api, 'bacs') on the previous API, but the diff does not show how CheckoutApmApi is wired into the main API — if CheckoutApmApi is shared between current and legacy API surfaces, the 'previous API must not expose bacs' guarantee cannot be confirmed from this truncated diff.
• In tests/accounts/accounts_integration_test.py, 8 integration tests are bulk-skipped with @pytest.mark.skip, with a reason attributing the breakage to a sandbox issue unrelated to this PR. Silently skipping 8 tests reduces coverage and any future regression in those paths would go undetected; these should either be fixed or tracked with a ticket reference rather than left as permanent skips.
• The renamed AchAccountType → AchSourceAccountType/AchInstrumentAccountType split and SepaMandateType local rename in setups.py to SetupsSepaMandateType are breaking changes for any downstream code importing these by name — the PR acknowledges breaking changes but only lists instrument model removals, not these enum renames.
• In instruments/instruments.py, the old InstrumentData class is removed and replaced with SepaInstrumentData/BacsInstrumentData/AchInstrumentData, but the diff is truncated so it is not possible to confirm that UpdateSepaInstrumentRequest, UpdateBacsInstrumentRequest, and UpdateAchInstrumentRequest are all present and correctly typed — the serialization test imports them, implying they exist, but cannot be fully verified.

⚠️ 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]

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

• exceeds_bounded_scope:343>200

Operational gates

• ✅ jira_ticket (INT-1675)
• ✅ independent_review

Files analysed: 12

---

_{wall-e 2026.06.19-02 · policy 376219bc71e6…}

[agent-wall-e]

🔬 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
exceeds_bounded_scope — 343>200	classifying	§2.1 M8	More than 200 non-test, non-doc, non-lockfile lines changed.

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]

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

• exceeds_bounded_scope:346>200

Operational gates

• ✅ jira_ticket (INT-1675)
• ✅ independent_review

Files analysed: 12

---

_{wall-e 2026.06.19-02 · policy 376219bc71e6…}

[agent-wall-e]

🔬 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
exceeds_bounded_scope — 346>200	classifying	§2.1 M8	More than 200 non-test, non-doc, non-lockfile lines changed.

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]

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

• exceeds_bounded_scope:425>200

Operational gates

• ✅ jira_ticket (INT-1675)
• ✅ independent_review

Files analysed: 13

---

_{wall-e 2026.06.19-02 · policy 376219bc71e6…}

[agent-wall-e]

🔬 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
exceeds_bounded_scope — 425>200	classifying	§2.1 M8	More than 200 non-test, non-doc, non-lockfile lines changed.

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]

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

• exceeds_bounded_scope:425>200

Operational gates

• ✅ jira_ticket (INT-1675)
• ✅ independent_review

Files analysed: 14

---

_{wall-e 2026.06.19-02 · policy 376219bc71e6…}

[agent-wall-e]

🔬 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
exceeds_bounded_scope — 425>200	classifying	§2.1 M8	More than 200 non-test, non-doc, non-lockfile lines changed.

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]

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

• exceeds_bounded_scope:452>200

Operational gates

• ✅ jira_ticket (INT-1675)
• ✅ independent_review

Files analysed: 16

---

_{wall-e 2026.06.19-02 · policy 376219bc71e6…}

[agent-wall-e]

🔬 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
exceeds_bounded_scope — 452>200	classifying	§2.1 M8	More than 200 non-test, non-doc, non-lockfile lines changed.

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]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud