From 045ab4e4087e52885656c8ab45ce51488616123c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 4 Jun 2026 15:58:10 +0200 Subject: [PATCH] fix(api/inventory): show account name in Commitments view (closes #952) resolveAccountNamesByID built its map keyed only by CloudAccount.ID (UUID), but PurchaseHistoryRecord.AccountID is the provider external ID (VARCHAR 20, e.g. an AWS account number), not the UUID. The lookup always missed, so AccountName was always empty and omitted from the JSON response, causing the frontend to fall through to the ID-only display path. Fix: also key the map by CloudAccount.ExternalID so the purchase-history lookup path resolves the display name correctly. UUID callers (recommendation records carrying cloud_account_id) continue to work via the UUID key. Update TestHandler_listActiveCommitments_FiltersExpired and AccountFilter to use realistic fixtures (ID=UUID, ExternalID=legacy account ID) so this regression cannot hide behind matching test values again. --- internal/api/handler_inventory_test.go | 13 +++++++++---- internal/api/scoping.go | 16 ++++++++++++---- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/internal/api/handler_inventory_test.go b/internal/api/handler_inventory_test.go index 05097078a..92f93691a 100644 --- a/internal/api/handler_inventory_test.go +++ b/internal/api/handler_inventory_test.go @@ -93,10 +93,15 @@ func TestHandler_listActiveCommitments_FiltersExpired(t *testing.T) { } mockStore.On("GetAllPurchaseHistory", ctx, config.MaxListLimit).Return(purchases, nil) + // Use realistic fixtures: CloudAccount.ID is a UUID; CloudAccount.ExternalID is + // the provider external ID (e.g. AWS account number) that matches + // PurchaseHistoryRecord.AccountID. The name lookup in resolveAccountNamesByID + // must key by ExternalID, not UUID, to find the name — this is the regression + // guard for issue #952. mockStore.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { return []config.CloudAccount{ - {ID: "acc-active", Name: "Active Account"}, - {ID: "acc-expired", Name: "Expired Account"}, + {ID: "11111111-0000-0000-0000-000000000001", ExternalID: "acc-active", Name: "Active Account"}, + {ID: "11111111-0000-0000-0000-000000000002", ExternalID: "acc-expired", Name: "Expired Account"}, }, nil } @@ -111,7 +116,7 @@ func TestHandler_listActiveCommitments_FiltersExpired(t *testing.T) { row := resp.Commitments[0] assert.Equal(t, "acc-active:p-active", row.ID, "id namespaces account+purchase") assert.Equal(t, "acc-active", row.AccountID) - assert.Equal(t, "Active Account", row.AccountName, "account name must be joined from ListCloudAccounts") + assert.Equal(t, "Active Account", row.AccountName, "account name must be resolved via ExternalID (issue #952)") assert.Equal(t, "aws", row.Provider) assert.Equal(t, "ec2", row.Service) assert.Equal(t, 2, row.Count) @@ -149,7 +154,7 @@ func TestHandler_listActiveCommitments_AccountFilter(t *testing.T) { mockStore.On("GetPurchaseHistory", ctx, "acc-1", config.MaxListLimit).Return(purchases, nil) mockStore.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { - return []config.CloudAccount{{ID: "acc-1", Name: "Account One"}}, nil + return []config.CloudAccount{{ID: "22222222-0000-0000-0000-000000000001", ExternalID: "acc-1", Name: "Account One"}}, nil } mockAuth, req := adminInventoryReq(ctx) diff --git a/internal/api/scoping.go b/internal/api/scoping.go index 00ae6109e..31580221b 100644 --- a/internal/api/scoping.go +++ b/internal/api/scoping.go @@ -144,9 +144,13 @@ func (h *Handler) requireExecutionAccess(ctx context.Context, session *Session, } // resolveAccountNamesByID fetches the complete account list once and returns -// a map from account ID → display name. Used by handlers that filter records -// (recommendations, purchases, history) by account ID but need name-based -// matching in auth.MatchesAccount. +// a map from account identifier → display name. The map is keyed by BOTH +// the internal UUID (CloudAccount.ID) and the cloud-provider external ID +// (CloudAccount.ExternalID, e.g. an AWS account number or Azure subscription +// ID). Dual-keying is necessary because different record types use different +// identifiers: recommendation records carry the UUID (cloud_account_id FK) +// while legacy purchase_history rows carry the external ID (account_id +// VARCHAR). Without both keys, the name lookup always misses for one path. // // Returns an empty map on error; callers fall through to ID-only matching, // which is still safe (MatchesAccount falls back to ID comparison). @@ -155,9 +159,13 @@ func (h *Handler) resolveAccountNamesByID(ctx context.Context) map[string]string if err != nil { return map[string]string{} } - nameByID := make(map[string]string, len(accounts)) + // Allocate 2× capacity since each account contributes two keys. + nameByID := make(map[string]string, len(accounts)*2) for _, a := range accounts { nameByID[a.ID] = a.Name + if a.ExternalID != "" { + nameByID[a.ExternalID] = a.Name + } } return nameByID }