From 4e89c6576b7479e08174f252f07e2a9bf52f9104 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 20 May 2026 18:45:30 +0200 Subject: [PATCH 1/2] feat(filters): wire provider/account subscriptions to RI Exchange tab (closes #186) The RI Exchange sub-tab was the only main tab that did not reload when the global provider/account filter changed. Dashboard, Recommendations, Plans, and History already subscribed to state.subscribeProvider / state.subscribeAccount; RI Exchange was silently skipped. Changes: - Import state module into riexchange.ts. - Add isRIExchangeSubtabActive() guard (mirrors the Recommendations tab pattern) so filter changes off-screen don't trigger unnecessary fetches. - setupRIExchangeHandlers now subscribes both state listeners; changes are coalesced via a queueMicrotask to avoid double-reloads when the topbar fires provider+account updates in sequence (same pattern as recommendations.ts). - Tests: 3 new assertions pin subscription registration, active-tab reload, and hidden-tab skip. --- frontend/src/__tests__/riexchange.test.ts | 98 +++++++++++++++++++++++ frontend/src/riexchange.ts | 37 ++++++++- 2 files changed, 134 insertions(+), 1 deletion(-) diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index d3839af17..eb6b074f3 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -20,6 +20,29 @@ jest.mock('../navigation', () => ({ switchSettingsSubTab: jest.fn(), })); +// Capture subscription callbacks so tests can fire them directly. +// These arrays are populated by the mock factory below. Declared with +// `let` so the reference is stable across the hoisted jest.mock call. +let _providerListeners: Array<() => void> = []; +let _accountListeners: Array<() => void> = []; +jest.mock('../state', () => ({ + subscribeProvider: jest.fn((cb: () => void) => { + // _providerListeners may not be initialised yet at hoist time — + // access it lazily via the closure over the outer `let`. + _providerListeners = _providerListeners ?? []; + _providerListeners.push(cb); + return () => undefined; + }), + subscribeAccount: jest.fn((cb: () => void) => { + _accountListeners = _accountListeners ?? []; + _accountListeners.push(cb); + return () => undefined; + }), + getCurrentProvider: jest.fn(() => 'aws'), + getCurrentAccountIDs: jest.fn(() => []), + getCurrentUser: jest.fn(() => ({ id: 'u', email: 'u@example.com', role: 'admin' })), +})); + import { fillQuoteFromRI, loadReshapeRecommendations, @@ -450,3 +473,78 @@ describe('⚙︎ Exchange settings deep-link', () => { expect(navigation.switchSettingsSubTab).toHaveBeenCalledWith('purchasing'); }); }); + +// issue #186: provider/account subscriptions on the RI Exchange tab +describe('RI Exchange filter subscriptions (issue #186)', () => { + let instancesEl: HTMLDivElement; + let recsEl: HTMLDivElement; + let historyEl: HTMLDivElement; + let riExchangePanel: HTMLDivElement; + + beforeEach(() => { + instancesEl = document.createElement('div'); + instancesEl.id = 'ri-exchange-instances-list'; + recsEl = document.createElement('div'); + recsEl.id = 'ri-exchange-recommendations-list'; + historyEl = document.createElement('div'); + historyEl.id = 'ri-exchange-history-list'; + // The sub-tab panel must exist and be visible for the guard to pass. + riExchangePanel = document.createElement('div'); + riExchangePanel.id = 'inventory-ri-exchange'; + document.body.append(instancesEl, recsEl, historyEl, riExchangePanel); + + (api.listConvertibleRIs as jest.Mock).mockResolvedValue([]); + (api.getRIUtilization as jest.Mock).mockResolvedValue([]); + (api.getReshapeRecommendations as jest.Mock).mockResolvedValue({ recommendations: [], recs_staleness: '', recs_collected_at: null }); + (api.getRIExchangeHistory as jest.Mock).mockResolvedValue([]); + _providerListeners.length = 0; + _accountListeners.length = 0; + // Re-apply the implementation after jest.resetAllMocks() from a prior + // describe block may have cleared it. + const stateMod = jest.requireMock('../state') as { + subscribeProvider: jest.Mock; + subscribeAccount: jest.Mock; + }; + stateMod.subscribeProvider.mockImplementation((cb: () => void) => { + _providerListeners.push(cb); + return () => undefined; + }); + stateMod.subscribeAccount.mockImplementation((cb: () => void) => { + _accountListeners.push(cb); + return () => undefined; + }); + }); + + afterEach(() => { + document.body.innerHTML = ''; + // Use clearAllMocks rather than resetAllMocks so the subscribeProvider/ + // subscribeAccount mock implementations (which push to _providerListeners) + // are preserved across tests in this block. + jest.clearAllMocks(); + }); + + it('setupRIExchangeHandlers registers subscribeProvider and subscribeAccount', () => { + const stateMod = jest.requireMock('../state'); + setupRIExchangeHandlers(); + expect(stateMod.subscribeProvider).toHaveBeenCalled(); + expect(stateMod.subscribeAccount).toHaveBeenCalled(); + }); + + it('a provider change triggers loadRIExchange when the sub-tab is active', async () => { + setupRIExchangeHandlers(); + // Fire the provider listener (simulates topbar provider change). + _providerListeners.forEach(cb => cb()); + // Flush the microtask queue: queueMicrotask fires after all pending + // micro-ticks; wrapping in a resolved promise ensures we drain it. + await new Promise(r => setTimeout(r, 0)); + expect(api.listConvertibleRIs).toHaveBeenCalled(); + }); + + it('a provider change does NOT trigger loadRIExchange when the sub-tab is hidden', async () => { + riExchangePanel.classList.add('hidden'); + setupRIExchangeHandlers(); + _providerListeners.forEach(cb => cb()); + await new Promise(r => setTimeout(r, 0)); + expect(api.listConvertibleRIs).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index af737394a..827929a07 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -4,6 +4,7 @@ */ import * as api from './api'; +import * as state from './state'; import { formatDate, formatDateTime, escapeHtml, formatCurrency } from './utils'; import { switchTab, switchSettingsSubTab } from './navigation'; import { confirmDialog } from './confirmDialog'; @@ -49,7 +50,25 @@ export async function loadRIExchange(): Promise { } /** - * Setup RI Exchange event handlers + * True when the RI Exchange sub-tab is the currently visible panel. + * The sub-tab panel id is "inventory-ri-exchange" (see index.html). + * Used by the provider/account change subscriptions below to avoid + * unnecessary fetches while the user is on a different tab. + */ +function isRIExchangeSubtabActive(): boolean { + const panel = document.getElementById('inventory-ri-exchange'); + return panel !== null && !panel.classList.contains('hidden'); +} + +/** + * Setup RI Exchange event handlers. + * + * Wires the refresh button, the settings deep-link, and + * provider/account state subscriptions so the convertible-RI list + * and reshape recommendations reload when the operator switches the + * global account filter (issue #186). An active-subtab guard + * mirrors the Recommendations tab pattern to avoid redundant fetches + * while the panel is off-screen. */ export function setupRIExchangeHandlers(): void { // Refresh button. Quote + execute flow lives in the per-row "Exchange" @@ -72,6 +91,22 @@ export function setupRIExchangeHandlers(): void { } }); } + + // issue #186: reload when the global provider/account filter changes + // so the RI Exchange tables stay consistent with the rest of the UI. + // Coalesce the two events into a single reload (provider change also + // fires an account change via the topbar-filters.ts clearing logic). + let reloadQueued = false; + const scheduleReload = (): void => { + if (!isRIExchangeSubtabActive() || reloadQueued) return; + reloadQueued = true; + queueMicrotask(() => { + reloadQueued = false; + if (isRIExchangeSubtabActive()) void loadRIExchange(); + }); + }; + state.subscribeProvider(scheduleReload); + state.subscribeAccount(scheduleReload); } // ────────────────────────────────────────────── From 84a344c44237a38fbd0515db114e8817e89d06a5 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 00:22:19 +0200 Subject: [PATCH 2/2] test(riexchange): add account-change and coalescing tests (PR #582, CR 4344791727) Address two CR nitpicks: verify subscribeAccount path triggers a reload when the sub-tab is active, and that queueMicrotask batches simultaneous provider+account changes into one loadRIExchange call. --- frontend/src/__tests__/riexchange.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index eb6b074f3..643252a58 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -547,4 +547,21 @@ describe('RI Exchange filter subscriptions (issue #186)', () => { await new Promise(r => setTimeout(r, 0)); expect(api.listConvertibleRIs).not.toHaveBeenCalled(); }); + + it('an account change triggers loadRIExchange when the sub-tab is active', async () => { + setupRIExchangeHandlers(); + _accountListeners.forEach(cb => cb()); + await new Promise(r => setTimeout(r, 0)); + expect(api.listConvertibleRIs).toHaveBeenCalled(); + }); + + it('coalesces provider and account changes into a single reload', async () => { + setupRIExchangeHandlers(); + // Simulate topbar filter cascade: provider change triggers account reset + _providerListeners.forEach(cb => cb()); + _accountListeners.forEach(cb => cb()); + await new Promise(r => setTimeout(r, 0)); + // Should be called once, not twice + expect(api.listConvertibleRIs).toHaveBeenCalledTimes(1); + }); });