From d666c4ce5152f28f4747afad837063ed1af3c1e4 Mon Sep 17 00:00:00 2001 From: Stephan D Date: Wed, 4 Mar 2026 18:52:43 +0100 Subject: [PATCH] fixed ledger account name propagation when creating ledger account --- .../internal/service/ledger/accounts.go | 39 ++++++++---- .../internal/service/ledger/accounts_test.go | 44 +++++++++++++- .../internal/service/ledger/topology.go | 17 +++++- api/ledger/storage/mongo/store/accounts.go | 60 ++++++++++++++++--- 4 files changed, 137 insertions(+), 23 deletions(-) diff --git a/api/ledger/internal/service/ledger/accounts.go b/api/ledger/internal/service/ledger/accounts.go index 6d007a99..e6892f64 100644 --- a/api/ledger/internal/service/ledger/accounts.go +++ b/api/ledger/internal/service/ledger/accounts.go @@ -29,6 +29,8 @@ type createAccountParams struct { modelRole account_role.AccountRole } +const defaultLedgerAccountName = "Ledger account" + // validateCreateAccountInput validates and normalizes all fields from the request. func validateCreateAccountInput(req *ledgerv1.CreateAccountRequest) (createAccountParams, error) { if req == nil { @@ -88,7 +90,17 @@ func (s *Service) createAccountResponder(_ context.Context, req *ledgerv1.Create return nil, err } - // Topology roles resolve to existing system accounts. + // Operating accounts are user-facing and can coexist with topology accounts. + // Ensure topology exists first, then create a dedicated account. + if p.modelRole == account_role.AccountRoleOperating { + if err := s.ensureLedgerTopology(ctx, p.orgRef, p.currency); err != nil { + recordAccountOperation("create", "error") + return nil, err + } + return s.persistNewAccount(ctx, p, req) + } + + // Other topology roles resolve to existing system accounts. if isRequiredTopologyRole(p.modelRole) { return s.resolveTopologyAccount(ctx, p.orgRef, p.currency, p.modelRole) } @@ -139,7 +151,7 @@ func (s *Service) persistNewAccount(ctx context.Context, p createAccountParams, if len(metadata) == 0 { metadata = nil } - describable := describableFromProto(req.GetDescribable()) + describable := ensureDefaultLedgerAccountName(describableFromProto(req.GetDescribable())) const maxCreateAttempts = 3 for attempt := 0; attempt < maxCreateAttempts; attempt++ { @@ -157,15 +169,8 @@ func (s *Service) persistNewAccount(ctx context.Context, p createAccountParams, return &ledgerv1.CreateAccountResponse{Account: toProtoAccount(account)}, nil } - if errors.Is(err, merrors.ErrDataConflict) { - existing, lookupErr := s.storage.Accounts().GetByRole(ctx, p.orgRef, p.currency, p.modelRole) - if lookupErr == nil && existing != nil { - recordAccountOperation("create", "success") - return &ledgerv1.CreateAccountResponse{Account: toProtoAccount(existing)}, nil - } - if attempt < maxCreateAttempts-1 { - continue - } + if errors.Is(err, merrors.ErrDataConflict) && attempt < maxCreateAttempts-1 { + continue } recordAccountOperation("create", "error") @@ -396,6 +401,18 @@ func describableFromProto(desc *describablev1.Describable) *pmodel.Describable { } } +func ensureDefaultLedgerAccountName(desc *pmodel.Describable) *pmodel.Describable { + if desc == nil { + return &pmodel.Describable{Name: defaultLedgerAccountName} + } + if strings.TrimSpace(desc.Name) != "" { + return desc + } + copy := *desc + copy.Name = defaultLedgerAccountName + return © +} + func describableToProto(desc pmodel.Describable) *describablev1.Describable { name := strings.TrimSpace(desc.Name) var description *string diff --git a/api/ledger/internal/service/ledger/accounts_test.go b/api/ledger/internal/service/ledger/accounts_test.go index 5ac656e4..69612be9 100644 --- a/api/ledger/internal/service/ledger/accounts_test.go +++ b/api/ledger/internal/service/ledger/accounts_test.go @@ -13,6 +13,7 @@ import ( "github.com/tech/sendico/pkg/merrors" pmodel "github.com/tech/sendico/pkg/model" "github.com/tech/sendico/pkg/model/account_role" + describablev1 "github.com/tech/sendico/pkg/proto/common/describable/v1" ledgerv1 "github.com/tech/sendico/pkg/proto/ledger/v1" ) @@ -184,12 +185,15 @@ func TestCreateAccountResponder_AutoCreatesSettlementAccount(t *testing.T) { // default role require.Equal(t, ledgerv1.AccountRole_ACCOUNT_ROLE_OPERATING, resp.Account.Role) require.Equal(t, "USD", resp.Account.Currency) + require.Equal(t, ledgerv1.AccountType_ACCOUNT_TYPE_LIABILITY, resp.Account.AccountType) + require.Equal(t, defaultLedgerAccountName, resp.Account.GetDescribable().GetName()) - // Expect: required roles + settlement - require.Len(t, accountStore.created, 5) + // Expect: required topology roles + dedicated operating account + require.Len(t, accountStore.created, 6) var settlement *pmodel.LedgerAccount var operating *pmodel.LedgerAccount + var operatingCount int roles := make(map[account_role.AccountRole]bool) for _, acc := range accountStore.created { @@ -199,6 +203,7 @@ func TestCreateAccountResponder_AutoCreatesSettlementAccount(t *testing.T) { settlement = acc } if acc.Role == account_role.AccountRoleOperating { + operatingCount++ operating = acc } @@ -212,12 +217,13 @@ func TestCreateAccountResponder_AutoCreatesSettlementAccount(t *testing.T) { require.NotNil(t, settlement) require.NotNil(t, operating) + require.Equal(t, 2, operatingCount) for _, role := range RequiredRolesV1 { require.True(t, roles[role]) } - // Responder must return the operating account it created/resolved. + // Responder returns the dedicated operating account created for this request. require.Equal(t, operating.AccountCode, resp.Account.AccountCode) require.Equal(t, operating.GetID().Hex(), resp.Account.LedgerAccountRef) @@ -235,6 +241,38 @@ func TestCreateAccountResponder_AutoCreatesSettlementAccount(t *testing.T) { require.Equal(t, "true", settlement.Metadata["system"]) } +func TestCreateAccountResponder_OperatingPreservesProvidedNameAndType(t *testing.T) { + t.Parallel() + + orgRef := bson.NewObjectID() + accountStore := &accountStoreStub{} + svc := &Service{ + logger: zap.NewNop(), + storage: &repositoryStub{accounts: accountStore}, + } + + req := &ledgerv1.CreateAccountRequest{ + OrganizationRef: orgRef.Hex(), + AccountType: ledgerv1.AccountType_ACCOUNT_TYPE_REVENUE, + Currency: "usd", + Role: ledgerv1.AccountRole_ACCOUNT_ROLE_OPERATING, + Describable: &describablev1.Describable{ + Name: "Incoming revenue", + }, + } + + resp, err := svc.createAccountResponder(context.Background(), req)(context.Background()) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Account) + + require.Equal(t, ledgerv1.AccountType_ACCOUNT_TYPE_REVENUE, resp.Account.AccountType) + require.Equal(t, "Incoming revenue", resp.Account.GetDescribable().GetName()) + + // Topology accounts + dedicated operating account. + require.Len(t, accountStore.created, 6) +} + func TestCreateAccountResponder_RetriesOnConflict(t *testing.T) { t.Parallel() diff --git a/api/ledger/internal/service/ledger/topology.go b/api/ledger/internal/service/ledger/topology.go index ca089ec4..01239452 100644 --- a/api/ledger/internal/service/ledger/topology.go +++ b/api/ledger/internal/service/ledger/topology.go @@ -70,9 +70,15 @@ func (s *Service) ensureRoleAccount(ctx context.Context, orgRef bson.ObjectID, c account, err := s.storage.Accounts().GetByRole(ctx, orgRef, normalizedCurrency, role) if err == nil { - return account, nil + if isSystemTaggedAccount(account) { + return account, nil + } + s.logger.Info("Found non-system account for topology role; creating missing system account", + mzap.ObjRef("organization_ref", orgRef), + zap.String("currency", normalizedCurrency), + zap.String("role", string(role))) } - if !errors.Is(err, storage.ErrAccountNotFound) { + if err != nil && !errors.Is(err, storage.ErrAccountNotFound) { s.logger.Warn("Failed to resolve ledger account by role", zap.Error(err), mzap.ObjRef("organization_ref", orgRef), zap.String("currency", normalizedCurrency), zap.String("role", string(role))) @@ -105,6 +111,13 @@ func (s *Service) ensureRoleAccount(ctx context.Context, orgRef bson.ObjectID, c return account, nil } +func isSystemTaggedAccount(account *pmodel.LedgerAccount) bool { + if account == nil || account.Metadata == nil { + return false + } + return strings.EqualFold(strings.TrimSpace(account.Metadata["system"]), "true") +} + func newSystemAccount(orgRef bson.ObjectID, currency string, role account_role.AccountRole) *pmodel.LedgerAccount { ref := bson.NewObjectID() account := &pmodel.LedgerAccount{ diff --git a/api/ledger/storage/mongo/store/accounts.go b/api/ledger/storage/mongo/store/accounts.go index d9484450..c1221343 100644 --- a/api/ledger/storage/mongo/store/accounts.go +++ b/api/ledger/storage/mongo/store/accounts.go @@ -7,6 +7,7 @@ import ( "github.com/tech/sendico/ledger/storage" "github.com/tech/sendico/pkg/db/repository" + "github.com/tech/sendico/pkg/db/repository/builder" ri "github.com/tech/sendico/pkg/db/repository/index" "github.com/tech/sendico/pkg/merrors" "github.com/tech/sendico/pkg/mlogger" @@ -24,6 +25,11 @@ type accountsStore struct { repo repository.Repository } +const ( + orgCurrencyRoleNonOperatingIndex = "org_currency_role_non_operating_unique" + orgCurrencyRoleSystemOperatingName = "org_currency_role_system_operating_unique" +) + func NewAccounts(logger mlogger.Logger, db *mongo.Database) (storage.AccountsStore, error) { repo := repository.CreateMongoRepository(db, mservice.LedgerAccounts) @@ -41,7 +47,7 @@ func NewAccounts(logger mlogger.Logger, db *mongo.Database) (storage.AccountsSto return nil, err } - // Create compound index on organizationRef + currency + role (unique) + // Keep role uniqueness for non-operating organization accounts. roleIndex := &ri.Definition{ Keys: []ri.Key{ {Field: "organizationRef", Sort: ri.Asc}, @@ -49,16 +55,36 @@ func NewAccounts(logger mlogger.Logger, db *mongo.Database) (storage.AccountsSto {Field: "role", Sort: ri.Asc}, }, Unique: true, - PartialFilter: repository.Filter( - "scope", - pkm.LedgerAccountScopeOrganization, - ), + Name: orgCurrencyRoleNonOperatingIndex, + PartialFilter: repository.Query(). + Filter(repository.Field("scope"), pkm.LedgerAccountScopeOrganization). + Comparison(repository.Field("role"), builder.Ne, account_role.AccountRoleOperating), } if err := repo.CreateIndex(roleIndex); err != nil { logger.Error("Failed to ensure accounts role index", zap.Error(err)) return nil, err } + // Ensure only one system-tagged operating role per organization/currency. + systemOperatingRoleIndex := &ri.Definition{ + Keys: []ri.Key{ + {Field: "organizationRef", Sort: ri.Asc}, + {Field: "currency", Sort: ri.Asc}, + {Field: "role", Sort: ri.Asc}, + {Field: "metadata.system", Sort: ri.Asc}, + }, + Unique: true, + Name: orgCurrencyRoleSystemOperatingName, + PartialFilter: repository.Query(). + Filter(repository.Field("scope"), pkm.LedgerAccountScopeOrganization). + Filter(repository.Field("role"), account_role.AccountRoleOperating). + Filter(repository.Field("metadata.system"), "true"), + } + if err := repo.CreateIndex(systemOperatingRoleIndex); err != nil { + logger.Error("Failed to ensure system operating role index", zap.Error(err)) + return nil, err + } + // Create compound index on scope + systemPurpose + currency (unique) for system accounts systemIndex := &ri.Definition{ Keys: []ri.Key{ @@ -182,14 +208,34 @@ func (a *accountsStore) GetByRole(ctx context.Context, orgRef bson.ObjectID, cur return nil, merrors.InvalidArgument("accountsStore: empty role") } + result := &pkm.LedgerAccount{} limit := int64(1) + + // Prefer topology/system-tagged account when present. + systemQuery := repository.Query(). + Filter(repository.Field("organizationRef"), orgRef). + Filter(repository.Field("currency"), currency). + Filter(repository.Field("role"), role). + Filter(repository.Field("scope"), pkm.LedgerAccountScopeOrganization). + Filter(repository.Field("metadata.system"), "true"). + Limit(&limit) + if err := a.repo.FindOneByFilter(ctx, systemQuery, result); err == nil { + a.logger.Debug("System account loaded by role", mzap.ObjRef("accountRef", *result.GetID()), + zap.String("currency", currency), zap.String("role", string(role))) + return result, nil + } else if !errors.Is(err, merrors.ErrNoData) { + a.logger.Warn("Failed to get account by role", zap.Error(err), mzap.ObjRef("organization_ref", orgRef), + zap.String("currency", currency), zap.String("role", string(role))) + return nil, err + } + + // Fallback to any organization account with the role. query := repository.Query(). Filter(repository.Field("organizationRef"), orgRef). Filter(repository.Field("currency"), currency). Filter(repository.Field("role"), role). + Filter(repository.Field("scope"), pkm.LedgerAccountScopeOrganization). Limit(&limit) - - result := &pkm.LedgerAccount{} if err := a.repo.FindOneByFilter(ctx, query, result); err != nil { if errors.Is(err, merrors.ErrNoData) { a.logger.Debug("Account not found by role", zap.String("currency", currency),