From 0063af5c915587620b46cc02f164a93e5817f641 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 17 Jul 2026 09:25:37 +0300 Subject: [PATCH 1/2] fix(ux): show account display name in Opportunities for non-admin users (#1419) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch loadRecommendations() from api.listAccounts() to api.listAccountsMinimal() to populate the accountNamesCache. listAccounts() calls GET /api/accounts which requires the admin view:accounts permission; non-admin users receive a 403, .catch(()=>[]) silences it, the cache stays empty, and every row falls back to showing the raw account UUID. listAccountsMinimal() calls GET /api/accounts/list which is gated on view:recommendations — the same permission that governs the whole Opportunities page — so standard users can resolve account names just like admin users. Add tests verifying listAccountsMinimal is called (not listAccounts), that the display name appears in the rendered row, and that the raw ID is still used as a fallback when no matching entry is found. Closes #1419 --- .../src/__tests__/recommendations.test.ts | 93 +++++++++++++++++++ frontend/src/recommendations.ts | 2 +- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 3ecae87f1..12d451084 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -9,6 +9,7 @@ jest.mock('../api', () => ({ getRecommendations: jest.fn(), refreshRecommendations: jest.fn(), listAccounts: jest.fn().mockResolvedValue([]), + listAccountsMinimal: jest.fn().mockResolvedValue([]), // issue #223: getConfig is fetched on page load to resolve GlobalConfig // defaults (DefaultTerm + DefaultPayment). Default-empty global config so // pre-#223 tests retain their hardcoded-fallback behavior without extra setup. @@ -7833,3 +7834,95 @@ describe('Issue #135: SP group parent row rendered in table', () => { expect(groupSavings!.textContent).toContain('$300'); }); }); + +// ============================================================================ +// Issue #1419: account display name shown for non-admin users +// ============================================================================ +// loadRecommendations must call listAccountsMinimal() (which hits +// /api/accounts/list, accessible to all users with view:recommendations) +// rather than listAccounts() (which hits /api/accounts, admin-only). +// Non-admin users previously saw the raw UUID because listAccounts() +// returned 403, .catch(() => []) silenced it, and the empty cache caused +// the ID to fall through as the display value. +describe('Issue #1419: Opportunities Account column shows display name via listAccountsMinimal', () => { + beforeEach(() => { + document.body.innerHTML = ` +
+
+
+
+ + `; + jest.clearAllMocks(); + jest.useFakeTimers(); + (state.getHiddenColumns as jest.Mock).mockReturnValue(new Set()); + (state.getRecommendationsSort as jest.Mock).mockReturnValue({ column: 'savings', direction: 'desc' }); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + (state.getCurrentUser as jest.Mock).mockReturnValue({ id: 'u-viewer', email: 'viewer@example.com', groups: [] }); + (recsApi.getRecommendationsFreshness as jest.Mock).mockResolvedValue({ + last_collected_at: new Date(Date.now() - 60 * 60 * 1000).toISOString(), + last_collection_error: null, + }); + (recsApi.refreshRecommendations as jest.Mock).mockResolvedValue({}); + }); + afterEach(() => { + jest.useRealTimers(); + }); + + test('listAccountsMinimal() is called (not listAccounts) so non-admin users get account names', async () => { + // Simulate listAccountsMinimal succeeding for a non-admin user. + (api.listAccountsMinimal as jest.Mock).mockResolvedValue([ + { id: 'acct-uuid-1234', name: 'Production AWS', external_id: '123456789', provider: 'aws' }, + ]); + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: {}, + recommendations: [{ + id: 'rec-acct', provider: 'aws', cloud_account_id: 'acct-uuid-1234', + service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', + count: 1, term: 1, payment: 'all-upfront', savings: 50, upfront_cost: 200, + }], + regions: [], + }); + + await loadRecommendations(); + + // listAccountsMinimal must have been called (not the admin-only listAccounts). + expect(api.listAccountsMinimal).toHaveBeenCalled(); + expect(api.listAccounts).not.toHaveBeenCalled(); + + const list = document.getElementById('recommendations-list'); + const html = list?.innerHTML ?? ''; + // The display name must appear in the rendered row. + expect(html).toContain('Production AWS'); + // The raw UUID must NOT appear as the cell value. + expect(html).not.toContain('acct-uuid-1234'); + }); + + test('falls back to account ID when listAccountsMinimal returns no matching entry', async () => { + (api.listAccountsMinimal as jest.Mock).mockResolvedValue([ + { id: 'acct-other', name: 'Other Account', external_id: '999', provider: 'aws' }, + ]); + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: {}, + recommendations: [{ + id: 'rec-fallback', provider: 'aws', cloud_account_id: 'acct-uuid-no-match', + service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', + count: 1, term: 1, payment: 'all-upfront', savings: 50, upfront_cost: 200, + }], + regions: [], + }); + + await loadRecommendations(); + + const list = document.getElementById('recommendations-list'); + const html = list?.innerHTML ?? ''; + // No matching entry: falls back to the raw ID. + expect(html).toContain('acct-uuid-no-match'); + }); +}); diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 10c4fd749..b3daf8e20 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -499,7 +499,7 @@ export async function loadRecommendations(): Promise { const [data, accounts, cfgResponse] = await Promise.all([ api.getRecommendations(filters) as unknown as RecommendationsResponse, - api.listAccounts().catch(() => []), + api.listAccountsMinimal().catch(() => []), // issue #223: fetch GlobalConfig so per-cell variant selection and // bulk-toolbar defaults reflect the operator's configured preference. // Failure is siloed — a missing/unreachable config endpoint must not From aa578be86ffd2907b515b34d85b49f18da95ecad Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 17 Jul 2026 09:53:43 +0300 Subject: [PATCH 2/2] test(recs): stub listAccountsMinimal in sibling recommendations suites (#1419) The #1419 fix switched loadRecommendations() to api.listAccountsMinimal(), but three sibling test suites mock ../api with listAccounts only. Without a listAccountsMinimal stub the call returns undefined, loadRecommendations throws before the toolbar/buttons render, and 38 assertions fail under the full jest run (CI Frontend/Unit). Add listAccountsMinimal: jest.fn().mockResolvedValue([]) to the ../api mock in recommendations-permissions, recommendations-enabled-providers, and recommendations-lookback so the full suite passes. --- frontend/src/__tests__/recommendations-enabled-providers.test.ts | 1 + frontend/src/__tests__/recommendations-lookback.test.ts | 1 + frontend/src/__tests__/recommendations-permissions.test.ts | 1 + 3 files changed, 3 insertions(+) diff --git a/frontend/src/__tests__/recommendations-enabled-providers.test.ts b/frontend/src/__tests__/recommendations-enabled-providers.test.ts index c232c109f..cd1680f88 100644 --- a/frontend/src/__tests__/recommendations-enabled-providers.test.ts +++ b/frontend/src/__tests__/recommendations-enabled-providers.test.ts @@ -15,6 +15,7 @@ jest.mock('../api', () => ({ getRecommendations: jest.fn(), refreshRecommendations: jest.fn(), listAccounts: jest.fn().mockResolvedValue([]), + listAccountsMinimal: jest.fn().mockResolvedValue([]), getConfig: jest.fn(), listAccountServiceOverrides: jest.fn().mockResolvedValue([]), })); diff --git a/frontend/src/__tests__/recommendations-lookback.test.ts b/frontend/src/__tests__/recommendations-lookback.test.ts index 3d05e71d6..a0090aab5 100644 --- a/frontend/src/__tests__/recommendations-lookback.test.ts +++ b/frontend/src/__tests__/recommendations-lookback.test.ts @@ -28,6 +28,7 @@ jest.mock('../api', () => ({ getConfig: jest.fn(), updateConfig: jest.fn().mockResolvedValue({}), listAccounts: jest.fn().mockResolvedValue([]), + listAccountsMinimal: jest.fn().mockResolvedValue([]), listAccountServiceOverrides: jest.fn().mockResolvedValue([]), })); diff --git a/frontend/src/__tests__/recommendations-permissions.test.ts b/frontend/src/__tests__/recommendations-permissions.test.ts index 3f1a95636..2a5233f1b 100644 --- a/frontend/src/__tests__/recommendations-permissions.test.ts +++ b/frontend/src/__tests__/recommendations-permissions.test.ts @@ -25,6 +25,7 @@ jest.mock('../api', () => ({ refreshRecommendations: jest.fn(), getConfig: jest.fn().mockResolvedValue({ global: {} }), listAccounts: jest.fn().mockResolvedValue([]), + listAccountsMinimal: jest.fn().mockResolvedValue([]), listAccountServiceOverrides: jest.fn().mockResolvedValue([]), }));