Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 185 additions & 2 deletions frontend/src/__tests__/recommendations.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Recommendations module tests
*/
import { loadRecommendations, openPurchaseModal, getPurchaseModalRecommendations, refreshRecommendations, setupRecommendationsHandlers, clearRecommendationDetailCache, pickBestVariantPerCell, seedGlobalDefaults, effectiveMonthlySavings, effectiveSavingsPct, groupRecsByCell, cellSummary, pageLevelRange, resetExpandedCells } from '../recommendations';
import { loadRecommendations, openPurchaseModal, getPurchaseModalRecommendations, refreshRecommendations, setupRecommendationsHandlers, clearRecommendationDetailCache, pickBestVariantPerCell, seedGlobalDefaults, effectiveMonthlySavings, effectiveSavingsPct, groupRecsByCell, cellSummary, pageLevelRange, resetExpandedCells, resetAutoRefreshInFlight } from '../recommendations';

// Mock the api module
jest.mock('../api', () => ({
Expand All @@ -25,14 +25,33 @@ jest.mock('../api', () => ({
// Default resolution returns a benign empty payload so tests that
// merely open + close the drawer (and don't care about the detail
// fetch) don't trip on an undefined-promise return.
//
// Default freshness: fresh (1h ago) so pre-#284 tests that call
// loadRecommendations() don't inadvertently trigger auto-refresh and
// fire extra showToast() calls that would break existing assertions.
// NOTE: ONE_HOUR_AGO can't be used here because jest.mock() factory
// functions are hoisted before variable declarations. The date is
// computed inline; the beforeEach block resets it via the mock handle.
jest.mock('../api/recommendations', () => ({
getRecommendationDetail: jest.fn().mockResolvedValue({
id: 'rec-default',
usage_history: [],
confidence_bucket: 'low',
provenance_note: '',
}),
getRecommendationsFreshness: jest.fn().mockResolvedValue({ last_collected_at: null, last_collection_error: null }),
getRecommendationsFreshness: jest.fn().mockResolvedValue({
last_collected_at: new Date(Date.now() - 60 * 60 * 1000).toISOString(),
last_collection_error: null,
}),
refreshRecommendations: jest.fn().mockResolvedValue({}),
}));

// Mock showToast so auto-refresh (#284) tests can assert on toast calls
// without touching the DOM. Returns a dismiss handle per the ToastHandle
// interface so callers that invoke handle.dismiss() don't crash.
const mockShowToast = jest.fn<{ dismiss: () => void }, [unknown]>(() => ({ dismiss: jest.fn() }));
jest.mock('../toast', () => ({
showToast: (opts: unknown) => mockShowToast(opts),
}));

// Mock state module
Expand Down Expand Up @@ -68,6 +87,7 @@ jest.mock('../utils', () => ({

import * as api from '../api';
import * as state from '../state';
import * as recsApi from '../api/recommendations';

describe('Recommendations Module', () => {
beforeEach(() => {
Expand All @@ -85,6 +105,15 @@ describe('Recommendations Module', () => {
jest.clearAllMocks();
jest.useFakeTimers();
window.alert = jest.fn();

// Default freshness for pre-#284 tests: fresh (1h ago) → auto-refresh
// does NOT fire, so existing tests are unaffected by the new toast calls.
(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({});
mockShowToast.mockReturnValue({ dismiss: jest.fn() });
});

afterEach(() => {
Expand Down Expand Up @@ -797,6 +826,160 @@ describe('Recommendations Module', () => {
});
});

describe('auto-refresh on page open (#284)', () => {
// Helper: set up the minimal DOM + api mock that loadRecommendations needs.
function mockGetRecs() {
(api.getRecommendations as jest.Mock).mockResolvedValue({
summary: {},
recommendations: [],
regions: [],
});
}

beforeEach(() => {
// Reset the dedup guard so each test starts with no refresh in flight.
resetAutoRefreshInFlight();
});

test('cold cache (null last_collected_at) — refresh fires + in-flight toast shown', async () => {
mockGetRecs();
(recsApi.getRecommendationsFreshness as jest.Mock).mockResolvedValue({
last_collected_at: null,
last_collection_error: null,
});

await loadRecommendations();
// Flush microtasks so the async refreshRecommendations call runs.
await Promise.resolve();

expect(recsApi.refreshRecommendations).toHaveBeenCalledTimes(1);
expect(mockShowToast).toHaveBeenCalledWith(
expect.objectContaining({ message: 'Refreshing recommendations…', kind: 'info' }),
);
});

test('stale cache (>24h ago) — refresh fires + in-flight toast shown', async () => {
mockGetRecs();
const twentyFiveHoursAgo = new Date(Date.now() - 25 * 60 * 60 * 1000).toISOString();
(recsApi.getRecommendationsFreshness as jest.Mock).mockResolvedValue({
last_collected_at: twentyFiveHoursAgo,
last_collection_error: null,
});

await loadRecommendations();
await Promise.resolve();

expect(recsApi.refreshRecommendations).toHaveBeenCalledTimes(1);
expect(mockShowToast).toHaveBeenCalledWith(
expect.objectContaining({ message: 'Refreshing recommendations…', kind: 'info' }),
);
});

test('fresh cache (<24h ago) — refresh does NOT fire + no toast', async () => {
mockGetRecs();
const oneHourAgo = new Date(Date.now() - 60 * 60 * 1000).toISOString();
(recsApi.getRecommendationsFreshness as jest.Mock).mockResolvedValue({
last_collected_at: oneHourAgo,
last_collection_error: null,
});

await loadRecommendations();
await Promise.resolve();

expect(recsApi.refreshRecommendations).not.toHaveBeenCalled();
expect(mockShowToast).not.toHaveBeenCalled();
});

test('cold cache + collection error — error toast surfaces the message', async () => {
mockGetRecs();
(recsApi.getRecommendationsFreshness as jest.Mock).mockResolvedValue({
last_collected_at: null,
last_collection_error: 'Provider X returned 403 Forbidden',
});

await loadRecommendations();
await Promise.resolve();

expect(mockShowToast).toHaveBeenCalledWith(
expect.objectContaining({
message: expect.stringContaining('Provider X returned 403 Forbidden'),
kind: 'error',
}),
);
});

test('refresh failure — error toast shown with message', async () => {
mockGetRecs();
(recsApi.getRecommendationsFreshness as jest.Mock).mockResolvedValue({
last_collected_at: null,
last_collection_error: null,
});
(recsApi.refreshRecommendations as jest.Mock).mockRejectedValue(new Error('Network timeout'));

await loadRecommendations();
// Two microtask ticks: one for the freshness call, one for the refresh rejection.
await Promise.resolve();
await Promise.resolve();

expect(mockShowToast).toHaveBeenCalledWith(
expect.objectContaining({
message: 'Recommendations refresh failed: Network timeout',
kind: 'error',
}),
);
});

test('dedup — concurrent stale loads fire refreshRecommendations only once', async () => {
mockGetRecs();
(recsApi.getRecommendationsFreshness as jest.Mock).mockResolvedValue({
last_collected_at: null,
last_collection_error: null,
});

// A pending promise that won't resolve until we call resolveRefresh().
let resolveRefresh!: () => void;
const pendingRefresh = new Promise<void>((r) => { resolveRefresh = r; });
(recsApi.refreshRecommendations as jest.Mock).mockReturnValue(pendingRefresh);

// Fire two stale loads without awaiting the first refresh to settle.
const first = loadRecommendations();
const second = loadRecommendations();

await first;
await second;
// Flush the microtasks that kick off triggerAutoRefreshIfStale.
await Promise.resolve();
await Promise.resolve();

// Only one API call despite two stale-load triggers.
expect(recsApi.refreshRecommendations).toHaveBeenCalledTimes(1);

// Clean up: resolve the pending promise so no timers hang after the test.
resolveRefresh();
await pendingRefresh;
});

test('persistent in-flight toast — shown with timeout: null so it stays until settled', async () => {
mockGetRecs();
(recsApi.getRecommendationsFreshness as jest.Mock).mockResolvedValue({
last_collected_at: null,
last_collection_error: null,
});

await loadRecommendations();
await Promise.resolve();

// Find the in-flight "Refreshing…" toast call and confirm it has no
// auto-dismiss timeout so it outlives long-running refreshes.
const inFlightCall = mockShowToast.mock.calls.find(
([opts]) => (opts as { message: string }).message === 'Refreshing recommendations…',
);
expect(inFlightCall).toBeDefined();
const inFlightOpts = inFlightCall![0] as { timeout?: number | null };
expect(inFlightOpts.timeout).toBeNull();
});
});

describe('openPurchaseModal', () => {
// Issue #111 (iii): openPurchaseModal is now async (it pre-fetches
// per-account service overrides to seed each row's Payment default).
Expand Down
1 change: 0 additions & 1 deletion frontend/src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ <h2>Upcoming Scheduled Purchases</h2>

<!-- Recommendations Tab -->
<div id="recommendations-tab" class="tab-content" role="tabpanel" aria-labelledby="recommendations-tab-btn">
<section id="recommendations-freshness" class="freshness-indicator" aria-live="polite"></section>
<section id="recommendations-summary"></section>
<!-- Per-column header filters replace the old top filter bar.
The Clear-filters badge + aria-live count are injected
Expand Down
118 changes: 112 additions & 6 deletions frontend/src/recommendations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
import * as api from './api';
import * as state from './state';
import { formatCurrency, formatTerm, escapeHtml } from './utils';
import { renderFreshness } from './freshness';
import { getRecommendationDetail, type RecommendationDetail } from './api/recommendations';
import { getRecommendationDetail, getRecommendationsFreshness, refreshRecommendations as refreshRecommendationsAPI, type RecommendationDetail } from './api/recommendations';
import { showToast } from './toast';
import {
isPaymentSupported,
Expand Down Expand Up @@ -41,6 +40,22 @@ let lastVisibleGroupKeys: string[] = [];
// in sync with the per-cell banner range under the table).
let lastRecommendationsSummary: RecommendationsSummary = {};

// #284 (CR follow-up): guard against concurrent stale-load refreshes. If a
// background refresh is already in flight, any subsequent stale detection
// skips kicking off a second request (and duplicate toasts).
let autoRefreshInFlight: Promise<void> | null = null;

/**
* Freshness budget for auto-refresh (#284). If the last successful collection
* is older than this threshold (or there has never been one), loadRecommendations
* fires a background refresh automatically so the user sees up-to-date data
* without needing an explicit Refresh button.
*
* TODO: expose via the /api/public-info endpoint so operators can tune this
* without a frontend redeploy.
*/
export const STALE_THRESHOLD_MS = 24 * 60 * 60 * 1000; // 24 hours

// issue #223: resolved GlobalConfig defaults, seeded on page load by
// loadRecommendations(). Initial values mirror the historical hardcoded
// defaults so the module is usable before the first API round-trip.
Expand All @@ -65,6 +80,14 @@ export function resetExpandedCells(): void {
lastVisibleGroupKeys = [];
}

/**
* Reset the auto-refresh in-flight guard. Exported for testing only — not
* part of the public API. Call in beforeEach so dedup tests start clean.
*/
export function resetAutoRefreshInFlight(): void {
autoRefreshInFlight = null;
}

// populateRecommendationsAccountFilter / populateRegionFilter / the legacy
// service-filter helpers were removed in Bundle B — those DOM elements are
// gone from index.html. Provider / Account values now drive an API re-fetch
Expand Down Expand Up @@ -99,6 +122,90 @@ export function setupRecommendationsHandlers(): void {
// commit hooks (see Bundle B follow-up).
}

/**
* Check freshness and fire a background refresh if the cache is stale or cold.
* Fire-and-forget: does not block the table render that already happened.
*
* Three toast stages:
* 1. In-flight: "Refreshing recommendations…" (sticky until resolved)
* 2. Success: "Recommendations refreshed"
* 3. Failure: "Recommendations refresh failed: <message>"
*
* If the freshness response itself carries a last_collection_error, that is
* surfaced as an error toast regardless of whether a new refresh fires (the
* freshness banner that previously showed this is being removed in #284).
*/
async function triggerAutoRefreshIfStale(): Promise<void> {
let freshness;
try {
freshness = await getRecommendationsFreshness();
} catch (err) {
// Network failures getting freshness are non-critical — the table is
// already rendered with the cached data; swallow silently.
console.error('Failed to fetch recommendations freshness:', err);
return;
}

const lastCollectedMs =
freshness.last_collected_at === null
? null
: new Date(freshness.last_collected_at).getTime();

const isStale =
lastCollectedMs === null ||
!Number.isFinite(lastCollectedMs) ||
Date.now() - lastCollectedMs > STALE_THRESHOLD_MS;
if (freshness.last_collection_error && isStale) {
showToast({
message: `Last recommendations collection had errors: ${freshness.last_collection_error}`,
kind: 'error',
});
}

if (!isStale) return;

// Dedup: if a refresh is already in flight, don't start a second one.
if (autoRefreshInFlight) return;

const inFlight = showToast({
message: 'Refreshing recommendations…',
kind: 'info',
// No auto-dismiss timeout — the .then()/.catch() paths call
// inFlight.dismiss() explicitly, so the toast stays visible until the
// refresh actually settles (real refreshes can take 28 s+).
timeout: null,
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

autoRefreshInFlight = refreshRecommendationsAPI()
.then(() => {
inFlight.dismiss();
showToast({
message: 'Recommendations refreshed',
kind: 'success',
timeout: 5_000,
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Reload UI data so users see fresh recommendations
return loadRecommendations();
})
.catch((err: unknown) => {
inFlight.dismiss();
const message =
err instanceof Error
? err.message
: err !== null && err !== undefined
? String(err)
: 'unknown error';
console.error('Auto-refresh of recommendations failed:', err);
showToast({
message: `Recommendations refresh failed: ${message}`,
kind: 'error',
});
})
.finally(() => {
autoRefreshInFlight = null;
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/**
* Load recommendations
*/
Expand Down Expand Up @@ -155,10 +262,9 @@ export async function loadRecommendations(): Promise<void> {
lastRecommendationsSummary = data.summary || {};
renderRecommendationsList(data.recommendations || []);

// Freshness indicator reflects the last collection timestamp; refreshed
// on every load so provider/account switches + manual refreshes stay
// in sync with the cache state.
void renderFreshness('recommendations-freshness', loadRecommendations);
// Auto-refresh on page open (#284): check freshness and trigger an
// async background refresh if the cache is cold or older than 24h.
void triggerAutoRefreshIfStale();
} catch (error) {
console.error('Failed to load recommendations:', error);
const list = document.getElementById('recommendations-list');
Expand Down