Skip to content

Commit 91f5e1c

Browse files
authored
refactor: update current account domain model for instrument-based quantities (#1213)
* refactor: update current account domain model for instrument-based quantities - Add instrumentCode and dimension fields with accessors InstrumentCode(), Dimension() - Rename accountIdentification to externalIdentifier; add ExternalIdentifier() accessor - Keep AccountIdentification() as deprecated alias for backward compatibility - Remove overdraft fields and methods (overdraftLimit, overdraftEnabled, overdraftRate, SetOverdraftLimit, UpdateOverdraftSettings); overdraft is now product-type behavior - Update NewCurrentAccount() constructor: currency string replaced by instrumentCode, dimension - Add NewCurrentAccountWithDimension() for explicit dimension specification - Update CurrentAccountBuilder with WithInstrumentCode(), WithDimension(), WithExternalIdentifier() - Update persistence layer: toDomain/toEntity use new fields, zero-out legacy overdraft columns - Update service layer: remove overdraft logic, use ExternalIdentifier() and InstrumentCode() - Update all saga orchestrators to use account.ExternalIdentifier() in saga inputs * fix: address CodeRabbit review findings in current account domain model - Clarify NewCurrentAccountWithDimension docstring: CURRENCY-only constraint is intentional for now (enforced by NewMoneyFromInstrument); non-currency support is deferred pending Money type generalisation in later tasks - Handle Compare error explicitly in PrepareForDebit and Withdraw instead of discarding with blank identifier; error is unreachable (currency already verified) but should not be silently swallowed * fix: address second round of CodeRabbit review findings - Fix NewCurrentAccount docblock: remove stale dimension parameter mention; document that dimension defaults to CURRENCY - Normalize dimension to uppercase in NewCurrentAccountWithDimension so Dimension() always returns canonical form (e.g. "currency" → "CURRENCY") - Normalize dimension to uppercase in CurrentAccountBuilder.WithDimension for consistency * docs: tighten current account domain comments to reflect CURRENCY-only constraint Remove kWh/ELECTRICITY examples from InstrumentCode, Dimension, and WithDimension docstrings; update inline docs to clearly state that only CURRENCY dimension is currently supported, avoiding misleading callers about non-currency capabilities --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent 45647e6 commit 91f5e1c

10 files changed

Lines changed: 298 additions & 596 deletions

File tree

services/current-account/adapters/persistence/repository.go

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -653,16 +653,16 @@ func toEntity(ctx context.Context, account domain.CurrentAccount) (*CurrentAccou
653653
// in-memory round-trip. Position Keeping is now the source of truth for balances.
654654
return &CurrentAccountEntity{
655655
ID: account.ID(),
656-
AccountID: account.AccountID(), // Business account identifier
657-
AccountIdentification: account.AccountIdentification(), // IBAN stored in account_identification
658-
AccountType: "current", // Default for current accounts
659-
InstrumentCode: account.Balance().CurrencyCode(),
660-
Dimension: account.Balance().Quantity().Instrument.Dimension,
656+
AccountID: account.AccountID(), // Business account identifier
657+
AccountIdentification: account.ExternalIdentifier(), // IBAN stored in account_identification
658+
AccountType: "current", // Default for current accounts
659+
InstrumentCode: account.InstrumentCode(),
660+
Dimension: account.Dimension(),
661661
Status: string(account.Status()),
662662
PartyID: partyUUID,
663663
OrgPartyID: account.OrgPartyID(),
664-
OverdraftLimit: account.OverdraftLimit().ToMinorUnitsUnchecked(),
665-
OverdraftRate: account.OverdraftRate(),
664+
OverdraftLimit: 0, // Overdraft is now product-type behavior, not domain state
665+
OverdraftRate: 0, // Overdraft is now product-type behavior, not domain state
666666
ProductTypeCode: productTypeCode,
667667
ProductTypeVersion: productTypeVersion,
668668
Balance: account.Balance().ToMinorUnitsUnchecked(), // gorm:"-" - not persisted
@@ -678,7 +678,6 @@ func toEntity(ctx context.Context, account domain.CurrentAccount) (*CurrentAccou
678678
}
679679

680680
// toDomain converts database entity to domain model using the builder pattern.
681-
// Note: OverdraftEnabled is derived from OverdraftLimit > 0
682681
// Note: Balance fields are not persisted - balance computation delegated to Position Keeping service.
683682
// The service layer is responsible for populating balance from Position Keeping after retrieval.
684683
func toDomain(entity *CurrentAccountEntity) (domain.CurrentAccount, error) {
@@ -695,14 +694,6 @@ func toDomain(entity *CurrentAccountEntity) (domain.CurrentAccount, error) {
695694
return domain.CurrentAccount{}, fmt.Errorf("failed to create available balance: %w", err)
696695
}
697696

698-
overdraftLimit, err := domain.NewMoneyFromInstrument(entity.InstrumentCode, entity.Dimension, entity.OverdraftLimit)
699-
if err != nil {
700-
return domain.CurrentAccount{}, fmt.Errorf("failed to create overdraft limit from database: %w", err)
701-
}
702-
703-
// Derive overdraft enabled from limit > 0
704-
overdraftEnabled := entity.OverdraftLimit > 0
705-
706697
// Balance is now computed by Position Keeping service, so use current time as placeholder.
707698
// The service layer will update this when fetching balance from Position Keeping.
708699
balanceUpdatedAt := entity.UpdatedAt
@@ -744,17 +735,16 @@ func toDomain(entity *CurrentAccountEntity) (domain.CurrentAccount, error) {
744735
return domain.NewCurrentAccountBuilder().
745736
WithID(entity.ID).
746737
WithAccountID(entity.AccountID).
747-
WithAccountIdentification(entity.AccountIdentification).
738+
WithExternalIdentifier(entity.AccountIdentification).
739+
WithInstrumentCode(entity.InstrumentCode).
740+
WithDimension(entity.Dimension).
748741
WithPartyID(entity.PartyID.String()).
749742
WithOrgPartyID(entity.OrgPartyID).
750743
WithBalance(balance).
751744
WithAvailableBalance(availableBalance).
752745
WithStatus(domain.AccountStatus(entity.Status)).
753746
WithFreezeReason(freezeReason).
754747
WithStatusHistory(statusHistory).
755-
WithOverdraftLimit(overdraftLimit).
756-
WithOverdraftEnabled(overdraftEnabled).
757-
WithOverdraftRate(entity.OverdraftRate).
758748
WithBalanceUpdatedAt(balanceUpdatedAt).
759749
WithVersion(entity.Version).
760750
WithCreatedAt(entity.CreatedAt).

services/current-account/adapters/persistence/repository_contract_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func TestFindByID_UsesAccountID(t *testing.T) {
4949
found, err := repo.FindByID(ctx, accountID)
5050
require.NoError(t, err, "FindByID with account_id should succeed")
5151
assert.Equal(t, accountID, found.AccountID())
52-
assert.Equal(t, accountIdentification, found.AccountIdentification())
52+
assert.Equal(t, accountIdentification, found.ExternalIdentifier())
5353

5454
// FindByID with IBAN should NOT find the account (it's not searching that column)
5555
_, err = repo.FindByID(ctx, accountIdentification)

services/current-account/adapters/persistence/repository_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,8 @@ func TestFindByIBAN(t *testing.T) {
207207
if retrieved.AccountID() != accountID {
208208
t.Errorf("Expected AccountID %s, got %s", accountID, retrieved.AccountID())
209209
}
210-
if retrieved.AccountIdentification() != iban {
211-
t.Errorf("Expected IBAN %s, got %s", iban, retrieved.AccountIdentification())
210+
if retrieved.ExternalIdentifier() != iban {
211+
t.Errorf("Expected IBAN %s, got %s", iban, retrieved.ExternalIdentifier())
212212
}
213213
}
214214

0 commit comments

Comments
 (0)