Skip to content

Release 8.1.0 - BACS Direct Debit notifications + refactor - #662

Merged
david-ruiz-cko merged 1 commit into
masterfrom
release/8.1.0
Sep 7, 2026
Merged

Release 8.1.0 - BACS Direct Debit notifications + refactor#662
david-ruiz-cko merged 1 commit into
masterfrom
release/8.1.0

Conversation

@david-ruiz-cko

Copy link
Copy Markdown
Contributor

Breaking changes (check at the end image)

This release introduces Bacs Direct Debit support to the SDK, enabling sending pre-notification (advance notice) events and full instrument lifecycle management for Bacs payment methods. The changes include new client interfaces and implementations, updates to serialization logic, and enhancements to the API surface. The most important changes are grouped below.

Bacs Direct Debit Client and API Enhancements:

  • Added a new BacsClient interface and its implementation BacsClientImpl to support sending Bacs Direct Debit pre-notification events, with both asynchronous and synchronous methods. This client is configured to use secret key authorization only. [1] [2]
  • Integrated the new BacsClient into the main API (AbstractCheckoutApmApi and CheckoutApmApi), making it accessible as bacsClient() alongside other APM clients. [1] [2] [3] [4]

Serialization and Instrument Support:

  • Updated the GsonSerializer to handle new Bacs-related classes, including request and response types for Bacs sources and instruments, ensuring correct (de)serialization of Bacs payment flows. This includes registering new instrument response subtypes for Bacs and updating the runtime type adapter factories to support polymorphic deserialization. [1] [2] [3] [4]

Documentation:

  • Added comprehensive documentation to README.md demonstrating how to use the new Bacs Direct Debit features, including sending pre-notifications, creating, updating, and retrieving Bacs instruments, and taking payments using stored Bacs instruments.

Breaking changes imageimageimage

Property moved off a base class

GetInstrumentResponse.accountHolder removed from the base, now on each concrete variant. Forced by Gson: two fields mapping to the same JSON name throw at runtime, and the spec gives all five retrieve variants a different account-holder shape. Base-typed getAccountHolder() no longer compiles and needs a cast to the concrete variant.

Changed field types

instruments.create.InstrumentData.paymentType: payments.PaymentType → SepaPaymentType. The old enum serialized "Recurring"/"Regular"; StoreSepaInstrumentRequest pins lowercase recurring/regular, so SEPA store could not send a valid value. Callers passing PaymentType.RECURRING must switch enum — though no such call can currently succeed against the API.
GetSepaInstrumentResponse.instrumentData: InstrumentData → GetSepaInstrumentData, which adds the mandate type (Core/B2B) the old type omitted.

Wire value changed

UpdateInstrumentBankAccountRequest sent "type":"token", now sends "type":"bank_account". token is absent from the update discriminator mapping entirely, so PATCH /instruments/{id} could not be validated as a bank-account update. No test referenced the class before this PR.

Deserialization behaviour changed (throw → object)

type: "bacs" and type: "ach" on the create and get instrument responses previously threw JsonParseException, because the three RuntimeTypeAdapterFactory registrations have no default subtype. They now deserialize to the concrete type. Anyone catching that exception as control flow is affected. The factories remain strict for unmodelled types — deliberately, per decision A2.

Deprecated, not removed (non-breaking)

PaymentSourceType.SEPA — zero callers; use ID for the previous platform, SEPAV4 for the current one
CreateInstrumentResponse.customer — moves to the token and bank_account variants in the next major
Correction to something I told you earlier
previous.RequestSepaSource switching from PaymentSourceType.SEPA to PaymentSourceType.ID is wire-neutral, not a breaking change. Both constants already carried @SerializedName("id") on master, so the bytes are identical. I listed it as a wire change twice, including in the ledger — now fixed there too.

@david-ruiz-cko
david-ruiz-cko requested a review from a team September 4, 2026 10:12
@agent-wall-e

agent-wall-e Bot commented Sep 4, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

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

Classification reasons

  • no_low_class_matched
  • prod_source_modified

Operational gates

  • ✅ jira_ticket
  • ✅ independent_review

Files analysed: 1


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Sep 4, 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
no_low_class_matched informational §2.2 (fall-through) None of the deterministic Low classes (§2.2.3, §2.2.4, §2.2.7, docs-only) applied; classifier fell through to LLM evaluation.
prod_source_modified informational §2.1 M7 (informational) At least one file is non-doc, non-test, non-IaC — i.e. application source code was modified.

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 4, 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.

The diff provided contains only a version bump from 8.0.0 to 8.1.0, but the PR description claims substantial breaking changes including new BacsClient interfaces, serialization logic changes, and multiple altered field types/behaviors. A human reviewer must verify that all claimed changes are actually present in the full PR and that the breaking changes described are intentional and acceptable.

For you to decide

  • The diff visible here is only the gradle.properties version increment — the entire substance of the PR (BacsClient, serialization changes, instrument response refactoring) is not present in what was provided for review, making it impossible to verify correctness of the implementation.
  • The PR explicitly lists multiple breaking changes (removed base-class fields, changed enum types, changed wire values, changed deserialization behavior) that must be reviewed against the project's versioning policy — if this is a semver-following project, breaking changes warrant a major version bump, not a minor one (8.1.0 vs 9.0.0).
  • The description acknowledges a wire value change in UpdateInstrumentBankAccountRequest ('token' → 'bank_account') that could silently break existing callers who were already using the class, even if no tests referenced it.
  • The deserialization behavior change (JsonParseException → concrete object for 'bacs' and 'ach' types) is a behavioral breaking change that could affect callers using exception-based control flow — this needs explicit human sign-off.

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

@sonarqubecloud

sonarqubecloud Bot commented Sep 4, 2026

Copy link
Copy Markdown

@david-ruiz-cko
david-ruiz-cko merged commit f4e5100 into master Sep 7, 2026
6 checks passed
@david-ruiz-cko
david-ruiz-cko deleted the release/8.1.0 branch September 7, 2026 08:56
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.

2 participants