Skip to content

feat: complete SDK endpoint coverage for account-sets and journals #3

@bmilleare

Description

@bmilleare

Summary

Add SDK methods for four endpoints currently in the OpenAPI spec but not exposed by the PHP client. Deferred from #2 (Codex review triage) because they are net-new feature additions rather than envelope-contract alignment.

Context

PR #2 brought the SDK in line with the backend's uniform-envelope contract and tightened the trancode and transaction surfaces, but Codex's review surfaced that the spec describes four endpoints we don't expose:

  • GET /api/v1/account-sets/{accountSetId}/balance
  • GET /api/v1/account-sets/{accountSetId}/accounts
  • POST /api/v1/account-sets/{accountSetId}/members
  • DELETE /api/v1/account-sets/{accountSetId}/members
  • GET /api/v1/journals/{journalId}/statistics

These are real backend features. SDK consumers currently have to bypass the SDK and hand-roll Guzzle calls to reach them.

Implementation Specification

Requirements

  • AccountSetService::getBalance(string $id): AccountSetBalance — wraps GET /account-sets/{id}/balance
  • AccountSetService::accounts(string $id): array — wraps GET /account-sets/{id}/accounts (recursively expands nested sets per the spec summary)
  • AccountSetService::addMember(string $id, string $memberType, string $memberId): void — wraps POST /account-sets/{id}/members
  • AccountSetService::removeMember(string $id, string $memberType, string $memberId): void — wraps DELETE /account-sets/{id}/members with a request body
  • JournalService::statistics(string $id): JournalStatistics — wraps GET /journals/{id}/statistics
  • New backed enum AccountSetMemberType (account, account_set) used by add/remove
  • New resource AccountSetBalance with: accountSetId, accountSetCode, accountSetName, totalBalance: string, accountCount: int, currency: string
  • New resource JournalStatistics with: totalTransactions: int, postedTransactions: int, pendingTransactions: int, reversedTransactions: int, totalAccountsAffected: int, lastTransactionDate: ?DateTimeImmutable

Technical Approach

Files to add:

  • src/Resources/AccountSetBalance.php (readonly DTO + fromArray())
  • src/Resources/JournalStatistics.php (readonly DTO + fromArray())
  • src/Enums/AccountSetMemberType.php (backed enum: account, account_set)
  • tests/Unit/Resources/AccountSetBalanceTest.php
  • tests/Unit/Resources/JournalStatisticsTest.php
  • New tests in AccountSetServiceTest and JournalServiceTest

Files to modify:

  • src/Services/AccountSetService.php — add the four methods
  • src/Services/JournalService.php — add statistics()
  • src/Http/HttpClientInterface.php — extend delete(string $path, array $data = []): Response to support a request body (DELETE with body is required by removeMember; current signature doesn't permit it)
  • src/Http/GuzzleHttpClient.php — implement the body-on-DELETE
  • tests/Unit/Http/GuzzleHttpClientTest.php — cover DELETE with body
  • README.md — examples for the new methods

API contracts (from openapi.json):

  • All five endpoints return the standard {success, data, ...} envelope and unwrap via the existing Response::unwrap() boundary
  • addMember and removeMember request body: {member_type, member_id} (both required, member_type[account, account_set])
  • accounts response is a list — return array<int, Account> reusing the existing resource, decoded via Account::fromArray over each item

Edge Cases

  • Empty account set: getBalance() should still return a valid AccountSetBalance with total_balance: "0.00" and account_count: 0.
  • accounts() on a deeply nested set: server expands recursively, SDK just decodes the flat array — confirm via live smoke that nesting depth doesn't break decoding.
  • statistics() on a journal with zero transactions: last_transaction_date will be null — must be ?DateTimeImmutable in the DTO.
  • DELETE-with-body via Guzzle: some HTTP intermediaries strip bodies on DELETE. Confirm the request reaches the server intact via live smoke; if not, fall back to query-string params.
  • Adding a member that already exists / removing one that doesn't: server behaviour TBD by live smoke; document the response in the method's docblock.

Success Criteria

Functionality

  • All five new SDK methods round-trip end-to-end against the demo ledger at 127.0.0.1:15080
  • addMember followed by accounts() shows the new member; removeMember followed by accounts() no longer shows it
  • getBalance() of a set with two accounts each containing distinct balances returns the sum
  • statistics() reflects an actual transaction count after a fresh transaction is posted

Test Coverage

  • Unit tests for each new service method (envelope unwrap shape pinned per the spec)
  • Unit tests for AccountSetMemberType enum (round-trip)
  • Unit tests for AccountSetBalance::fromArray and JournalStatistics::fromArray including nullable/missing fields
  • Integration smoke verifies all five endpoints against the live demo ledger
  • Edge cases listed above have explicit test coverage

Code Quality

  • PHPStan level 7 clean
  • PSR-12 / PHPCS clean
  • Follows existing SDK conventions: readonly resource DTOs implementing ResourceInterface, backed enums for finite value sets, services extending AbstractService<T> where applicable
  • README updated with account-sets (members + balance + accounts) and journals (statistics) examples
  • Docblocks describe contract semantics, not implementation narrative

Security (if applicable)

  • DELETE-with-body change to HttpClientInterface does not introduce regressions on existing delete callers (accounts->delete, account-sets->delete, journals->delete); existing tests still pass
  • No leakage of sensitive fields in error paths (the existing LedgaException hierarchy already handles this — verify nothing new short-circuits it)

Out of Scope

  • Trancode validate-params already shipped in Align SDK with v0.2 uniform-envelope API contract #2; not part of this work.
  • Other endpoints not currently in the spec at the time of this issue (e.g. any future reactivate for trancodes).
  • Refactoring of the existing AccountSet resource or AccountSetService — only additive.
  • Mass-update / batch operations on members.

Pre-PR Security Review

Before opening a PR, perform an OWASP analysis of all changes:

  • Review against OWASP Top 10 (injection, broken auth, sensitive data exposure, XXE, broken access control, security misconfiguration, XSS, insecure deserialization, vulnerable components, insufficient logging)
  • Document any identified risks and mitigations in the PR body
  • Confirm the DELETE-with-body change doesn't open a request-smuggling vector through any intermediate proxies in deployment
  • Confirm no new attack vectors introduced

Reference

  • Codex review thread on Align SDK with v0.2 uniform-envelope API contract #2 (PR comment dated 2026-04-28) for the full list of deferred items and their justification.
  • openapi.json paths /api/v1/account-sets/{accountSetId}/balance, /accounts, /members, and /api/v1/journals/{journalId}/statistics for canonical request/response shapes.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions