Merge pull request 'fixed ledger account name propagation when creating ledger account' (#642) from ledger-614 into main

Reviewed-on: #642

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) {
+		if errors.Is(err, merrors.ErrDataConflict) && attempt < maxCreateAttempts-1 {
-			existing, lookupErr := s.storage.Accounts().GetByRole(ctx, p.orgRef, p.currency, p.modelRole)
+			continue
-			if lookupErr == nil && existing != nil {
-				recordAccountOperation("create", "success")
-				return &ledgerv1.CreateAccountResponse{Account: toProtoAccount(existing)}, nil
-			}
-			if 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 &copy
+}
+
 func describableToProto(desc pmodel.Describable) *describablev1.Describable {
 	name := strings.TrimSpace(desc.Name)
 	var description *string

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
+	// Expect: required topology roles + dedicated operating account
-	require.Len(t, accountStore.created, 5)
+	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()
 

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{

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(
+		Name:   orgCurrencyRoleNonOperatingIndex,
-			"scope",
+		PartialFilter: repository.Query().
-			pkm.LedgerAccountScopeOrganization,
+			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),