Skip to content

feature/INT-1692 - Top-up instructions and amount allocations - #664

Merged
david-ruiz-cko merged 2 commits into
masterfrom
feature/INT-1692
Sep 10, 2026
Merged

feature/INT-1692 - Top-up instructions and amount allocations#664
david-ruiz-cko merged 2 commits into
masterfrom
feature/INT-1692

Conversation

@david-ruiz-cko

Copy link
Copy Markdown
Contributor

This pull request adds support for retrieving top-up instructions for currency accounts (sub-accounts) via the balances API, including both asynchronous and synchronous methods. It introduces new data models for top-up instructions and bank details, updates the OAuth scopes, and improves documentation across balances-related classes. Comprehensive unit tests for the new functionality are also included.

New API Functionality:

  • Added asynchronous and synchronous methods to BalancesClient and BalancesClientImpl for retrieving top-up instructions (retrieveTopUpInstructions and retrieveTopUpInstructionsSync), including input validation and endpoint path construction. [1] [2]
  • Introduced new response and model classes: TopUpInstructionsResponse, TopUpBankDetails, and TopUpFundingDetails to represent top-up instructions and associated bank details. [1] [2] [3]

Authorization & Scopes:

  • Added the BALANCES_TOP_UP_INSTRUCTIONS OAuth scope to both the enum and test fixture setup to support the new endpoint. [1] [2]

Documentation Improvements:

  • Enhanced Javadoc comments for Balances, BalancesResponse, and CurrencyAccountBalance to clarify the purpose and optionality of fields. [1] [2] [3] [4]

Testing:

  • Expanded BalancesClientImplTest to cover the new top-up instructions methods, including positive tests and parameterized negative tests for input validation. [1] [2] [3] [4] [5]

@david-ruiz-cko
david-ruiz-cko requested a review from a team September 7, 2026 11:17
@agent-wall-e

agent-wall-e Bot commented Sep 7, 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

  • security_sensitive_path:src/main/java/com/checkout/OAuthScope.java

Operational gates

  • ✅ jira_ticket (INT-1692)
  • ✅ independent_review

Files analysed: 13


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Sep 7, 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
security_sensitive_pathsrc/main/java/com/checkout/OAuthScope.java classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

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 7, 2026

Copy link
Copy Markdown

🟢 Advisory review: Looks good to me

This PR still needs a human approval — wall-e cannot auto-approve it. For what it's worth, I read the diff and found nothing I'd block on.

Adds a retrieveTopUpInstructions endpoint to the balances client with new model classes, an OAuth scope, and matching tests. The implementation is correct and consistent with existing patterns in the codebase.

What I checked

  • The URL path built by topUpInstructionsPath (entities/{entityId}/currency-accounts/{currencyAccountId}/top-up-instructions) matches the path documented in the IT test's Javadoc and the interface Javadoc.
  • Input validation via validateParams is applied symmetrically to both the async and sync methods before making the HTTP call.
  • The new operational field added to Balances is a purely additive, optional field with no default — it will deserialize as null for existing API responses that don't include it, which is safe.
  • The OAuthScopeTest pinning the wire value "balances:top-up-instructions" is a useful regression guard given the comment explaining that a scope typo silently drops all scopes from a token request.
  • The IT test's use of retrieveEntityBalancesSync with withCurrencyAccountId(true) assumes that method and that query parameter exist and work correctly — this is relying on existing tested code, so it's fine, but the diff is partial so the BalancesQuery.withCurrencyAccountId method is not visible here.
  • The IT test's silent early-return when no currencyAccountId is found (if (currencyAccountId == null) { return; }) means the top-up path is untested in that scenario, but this is a reasonable tradeoff for a sandbox environment and is clearly documented in the Javadoc.
  • The TopUpInstructionsResponse Javadoc marks bankDetails as [Required] while TopUpBankDetails itself documents that neither domestic nor international is guaranteed — this is internally consistent since the container object is required but its contents are optional.
  • Serialization tests in BalancesSerializationTest cover both directions (serialize + deserialize) and boundary cases (domestic-only, international-only, empty bank details), which is thorough coverage for the new model classes.

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

Comment thread src/main/java/com/checkout/balances/TopUpInstructionsResponse.java Dismissed
@agent-wall-e

agent-wall-e Bot commented Sep 9, 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

  • security_sensitive_path:src/main/java/com/checkout/OAuthScope.java
  • security_sensitive_path:src/test/java/com/checkout/OAuthScopeTest.java

Operational gates

  • ✅ jira_ticket (INT-1692)
  • ✅ independent_review

Files analysed: 14


wall-e 2026.06.19-02 · policy 376219bc71e6…

@agent-wall-e

agent-wall-e Bot commented Sep 9, 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
security_sensitive_pathsrc/main/java/com/checkout/OAuthScope.java classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).
security_sensitive_pathsrc/test/java/com/checkout/OAuthScopeTest.java classifying §2.1 M4/M5 Path matched a sensitive pattern (auth, secrets, crypto, PCI, migrations, network IaC).

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 9, 2026

Copy link
Copy Markdown

@david-ruiz-cko
david-ruiz-cko merged commit 78d3cf9 into master Sep 10, 2026
6 checks passed
@david-ruiz-cko
david-ruiz-cko deleted the feature/INT-1692 branch September 10, 2026 09:14
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.

3 participants