fixed ledger account name propagation when creating ledger account
This commit is contained in:
@@ -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,16 +169,9 @@ 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 {
|
||||
if errors.Is(err, merrors.ErrDataConflict) && attempt < maxCreateAttempts-1 {
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
recordAccountOperation("create", "error")
|
||||
s.logger.Warn("Failed to create account", zap.Error(err),
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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 {
|
||||
if isSystemTaggedAccount(account) {
|
||||
return account, nil
|
||||
}
|
||||
if !errors.Is(err, storage.ErrAccountNotFound) {
|
||||
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 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{
|
||||
|
||||
@@ -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),
|
||||
|
||||
Reference in New Issue
Block a user